[C] Add (new) -Wimplicit-void-ptr-cast to -Wc++-compat (llvm#136855)

This introduces a new diagnostic group (-Wimplicit-void-ptr-cast),
grouped under -Wc++-compat, which diagnoses implicit conversions from
void * to another pointer type in C. It's a common source of
incompatibility with C++ and is something GCC diagnoses (though GCC does
not have a specific warning group for this).

Fixes llvm#17792

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -140,6 +140,9 @@ C Language Changes
 - Clang now allows an ``inline`` specifier on a typedef declaration of a
   function type in Microsoft compatibility mode. #GH124869
 - Clang now allows ``restrict`` qualifier for array types with pointer elements (#GH92847).
+- Added ``-Wimplicit-void-ptr-cast``, grouped under ``-Wc++-compat``, which
+  diagnoses implicit conversion from ``void *`` to another pointer type as
+  being incompatible with C++. (#GH17792)
 
 C2y Feature Support
 ^^^^^^^^^^^^^^^^^^^

clang/include/clang/Basic/DiagnosticGroups.td

@@ -155,7 +155,8 @@ def C99Compat : DiagGroup<"c99-compat">;
 def C23Compat : DiagGroup<"c23-compat">;
 def : DiagGroup<"c2x-compat", [C23Compat]>;
 
-def CXXCompat: DiagGroup<"c++-compat">;
+def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">;
+def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast]>;
 def ExternCCompat : DiagGroup<"extern-c-compat">;
 def KeywordCompat : DiagGroup<"keyword-compat">;
 def GNUCaseRange : DiagGroup<"gnu-case-range">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8687,7 +8687,17 @@ def err_typecheck_missing_return_type_incompatible : Error<
   "%diff{return type $ must match previous return type $|"
   "return type must match previous return type}0,1 when %select{block "
   "literal|lambda expression}2 has unspecified explicit return type">;
-
+def warn_compatible_implicit_pointer_conv : Warning<
+  "implicit conversion when %select{"
+  "%diff{assigning to $ from type $|assigning to type from type}0,1|"
+  "%diff{passing $ to parameter of type $|passing type to parameter of type}0,1|"
+  "%diff{returning $ from a function with result type $|returning type from a function with result type}0,1|"
+  "<CLANG BUG IF YOU SEE THIS>|" // converting
+  "%diff{initializing $ with an expression of type $|initializing type with an expression of type}0,1|"
+  "%diff{sending $ to parameter of type $|sending type to parameter of type}0,1|"
+  "<CLANG BUG IF YOU SEE THIS>" // casting
+  "}2 is not permitted in C++">,
+  InGroup<ImplicitVoidPtrCast>, DefaultIgnore;
 def note_incomplete_class_and_qualified_id : Note<
   "conformance of forward class %0 to protocol %1 cannot be confirmed">;
 def warn_incompatible_qualified_id : Warning<

clang/include/clang/Sema/Sema.h

@@ -7786,6 +7786,11 @@ class Sema final : public SemaBase {
     /// Compatible - the types are compatible according to the standard.
     Compatible,
 
+    /// CompatibleVoidPtrToNonVoidPtr - The types are compatible in C because
+    /// a void * can implicitly convert to another pointer type, which we
+    /// differentiate for better diagnostic behavior.
+    CompatibleVoidPtrToNonVoidPtr,
+
     /// PointerToInt - The assignment converts a pointer to an int, which we
     /// accept as an extension.
     PointerToInt,
@@ -7866,6 +7871,18 @@ class Sema final : public SemaBase {
     Incompatible
   };
 
+  bool IsAssignConvertCompatible(AssignConvertType ConvTy) {
+    switch (ConvTy) {
+    default:
+      return false;
+    case Compatible:
+    case CompatiblePointerDiscardsQualifiers:
+    case CompatibleVoidPtrToNonVoidPtr:
+      return true;
+    }
+    llvm_unreachable("impossible");
+  }
+
   /// DiagnoseAssignmentResult - Emit a diagnostic, if required, for the
   /// assignment conversion type specified by ConvTy.  This returns true if the
   /// conversion was invalid or false if the conversion was accepted.

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3589,8 +3589,8 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // If this ever proves to be a problem it should be easy to fix.
   QualType Ty = S.Context.getPointerType(cast<VarDecl>(D)->getType());
   QualType ParamTy = FD->getParamDecl(0)->getType();
-  if (S.CheckAssignmentConstraints(FD->getParamDecl(0)->getLocation(),
-                                   ParamTy, Ty) != Sema::Compatible) {
+  if (!S.IsAssignConvertCompatible(S.CheckAssignmentConstraints(
+          FD->getParamDecl(0)->getLocation(), ParamTy, Ty))) {
     S.Diag(Loc, diag::err_attribute_cleanup_func_arg_incompatible_type)
       << NI.getName() << ParamTy << Ty;
     return;

clang/lib/Sema/SemaExpr.cpp

@@ -9062,8 +9062,12 @@ checkPointerTypesForAssignment(Sema &S, QualType LHSType, QualType RHSType,
   }
 
   if (rhptee->isVoidType()) {
+    // In C, void * to another pointer type is compatible, but we want to note
+    // that there will be an implicit conversion happening here.
     if (lhptee->isIncompleteOrObjectType())
-      return ConvTy;
+      return ConvTy == Sema::Compatible && !S.getLangOpts().CPlusPlus
+                 ? Sema::CompatibleVoidPtrToNonVoidPtr
+                 : ConvTy;
 
     // As an extension, we allow cast to/from void* to function pointer.
     assert(lhptee->isFunctionType());
@@ -9098,7 +9102,7 @@ checkPointerTypesForAssignment(Sema &S, QualType LHSType, QualType RHSType,
       // Types are compatible ignoring the sign. Qualifier incompatibility
       // takes priority over sign incompatibility because the sign
       // warning can be disabled.
-      if (ConvTy != Sema::Compatible)
+      if (!S.IsAssignConvertCompatible(ConvTy))
         return ConvTy;
 
       return Sema::IncompatiblePointerSign;
@@ -16980,7 +16984,11 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
   case Compatible:
       DiagnoseAssignmentEnum(DstType, SrcType, SrcExpr);
       return false;
-
+  case CompatibleVoidPtrToNonVoidPtr:
+    // Still a valid conversion, but we may want to diagnose for C++
+    // compatibility reasons.
+    DiagKind = diag::warn_compatible_implicit_pointer_conv;
+    break;
   case PointerToInt:
     if (getLangOpts().CPlusPlus) {
       DiagKind = diag::err_typecheck_convert_pointer_int;

clang/lib/Sema/SemaInit.cpp

@@ -8328,10 +8328,9 @@ ExprResult InitializationSequence::Perform(Sema &S,
 
       // If this is a call, allow conversion to a transparent union.
       ExprResult CurInitExprRes = CurInit;
-      if (ConvTy != Sema::Compatible &&
-          Entity.isParameterKind() &&
-          S.CheckTransparentUnionArgumentConstraints(Step->Type, CurInitExprRes)
-            == Sema::Compatible)
+      if (!S.IsAssignConvertCompatible(ConvTy) && Entity.isParameterKind() &&
+          S.CheckTransparentUnionArgumentConstraints(
+              Step->Type, CurInitExprRes) == Sema::Compatible)
         ConvTy = Sema::Compatible;
       if (CurInitExprRes.isInvalid())
         return ExprError();

clang/lib/Sema/SemaObjC.cpp

@@ -2341,8 +2341,8 @@ static void checkCollectionLiteralElement(Sema &S, QualType TargetElementType,
   QualType ElementType = Element->getType();
   ExprResult ElementResult(Element);
   if (ElementType->getAs<ObjCObjectPointerType>() &&
-      S.CheckSingleAssignmentConstraints(TargetElementType, ElementResult,
-                                         false, false) != Sema::Compatible) {
+      !S.IsAssignConvertCompatible(S.CheckSingleAssignmentConstraints(
+          TargetElementType, ElementResult, false, false))) {
     S.Diag(Element->getBeginLoc(), diag::warn_objc_collection_literal_element)
         << ElementType << ElementKind << TargetElementType
         << Element->getSourceRange();

clang/lib/Sema/SemaObjCProperty.cpp

@@ -1349,9 +1349,9 @@ Decl *SemaObjC::ActOnPropertyImplDecl(
             PropertyIvarType->castAs<ObjCObjectPointerType>(),
             IvarType->castAs<ObjCObjectPointerType>());
       else {
-        compat = (SemaRef.CheckAssignmentConstraints(
-                      PropertyIvarLoc, PropertyIvarType, IvarType) ==
-                  Sema::Compatible);
+        compat = SemaRef.IsAssignConvertCompatible(
+            SemaRef.CheckAssignmentConstraints(PropertyIvarLoc,
+                                               PropertyIvarType, IvarType));
       }
       if (!compat) {
         Diag(PropertyDiagLoc, diag::err_property_ivar_type)
@@ -1702,8 +1702,9 @@ bool SemaObjC::DiagnosePropertyAccessorMismatch(ObjCPropertyDecl *property,
              PropertyRValueType->getAs<ObjCObjectPointerType>()) &&
         (getterObjCPtr = GetterType->getAs<ObjCObjectPointerType>()))
       compat = Context.canAssignObjCInterfaces(getterObjCPtr, propertyObjCPtr);
-    else if (SemaRef.CheckAssignmentConstraints(
-                 Loc, GetterType, PropertyRValueType) != Sema::Compatible) {
+    else if (!SemaRef.IsAssignConvertCompatible(
+                 SemaRef.CheckAssignmentConstraints(Loc, GetterType,
+                                                    PropertyRValueType))) {
       Diag(Loc, diag::err_property_accessor_type)
           << property->getDeclName() << PropertyRValueType
           << GetterMethod->getSelector() << GetterType;

clang/lib/Sema/SemaOverload.cpp

@@ -2518,6 +2518,7 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
   ImplicitConversionKind SecondConv;
   switch (Conv) {
   case Sema::Compatible:
+  case Sema::CompatibleVoidPtrToNonVoidPtr: // __attribute__((overloadable))
     SecondConv = ICK_C_Only_Conversion;
     break;
   // For our purposes, discarding qualifiers is just as bad as using an

clang/test/Sema/implicit-void-ptr-cast.c

@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=c -Wimplicit-void-ptr-cast %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c -Wc++-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify=cxx -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify=good %s
+// RUN: %clang_cc1 -fsyntax-only -verify=good -Wc++-compat -Wno-implicit-void-ptr-cast %s
+// good-no-diagnostics
+
+typedef __typeof__(sizeof(int)) size_t;
+extern void *malloc(size_t);
+
+void func(int *); // #func-param
+
+void test(void) {
+  int *x = malloc(sizeof(char)); // c-warning {{implicit conversion when initializing 'int *' with an expression of type 'void *' is not permitted in C++}} \
+                                    cxx-error {{cannot initialize a variable of type 'int *' with an rvalue of type 'void *'}}
+  x = malloc(sizeof(char));      // c-warning {{implicit conversion when assigning to 'int *' from type 'void *' is not permitted in C++}} \
+                                    cxx-error {{assigning to 'int *' from incompatible type 'void *'}}
+  func(malloc(sizeof(char)));    // c-warning {{implicit conversion when passing 'void *' to parameter of type 'int *' is not permitted in C++}} \
+                                    c-note@#func-param {{passing argument to parameter here}} \
+                                    cxx-error {{no matching function for call to 'func'}} \
+                                    cxx-note@#func-param {{candidate function not viable: cannot convert argument of incomplete type 'void *' to 'int *' for 1st argument}}
+  x = (int *)malloc(sizeof(char));
+
+  void *vp = 0;
+  x = vp; // c-warning {{implicit conversion when assigning to 'int *' from type 'void *' is not permitted in C++}} \
+             cxx-error {{assigning to 'int *' from incompatible type 'void *'}}
+  vp = vp;
+
+  x = (void *)malloc(sizeof(char)); // c-warning {{implicit conversion when assigning to 'int *' from type 'void *' is not permitted in C++}} \
+                                       cxx-error {{assigning to 'int *' from incompatible type 'void *'}}
+  const int *y = vp;                // c-warning {{implicit conversion when initializing 'const int *' with an expression of type 'void *' is not permitted in C++}} \
+                                       cxx-error {{cannot initialize a variable of type 'const int *' with an lvalue of type 'void *'}}
+}
+
+int *other_func(void *ptr) {
+  return ptr; // c-warning {{implicit conversion when returning 'void *' from a function with result type 'int *' is not permitted in C++}} \
+                 cxx-error {{cannot initialize return object of type 'int *' with an lvalue of type 'void *'}}
+}