[C] Update -Wimplicit-void-ptr-cast for null pointer constants

[AaronBallman]

Null pointer constants require a bit of extra effort to handle in C.

Fixes #138145

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Null pointer constants require a bit of extra effort to handle in C.

Fixes #138145

---

Full diff: https://github.com/llvm/llvm-project/pull/138271.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaExpr.cpp (+25-6)
• (modified) clang/test/Sema/implicit-void-ptr-cast.c (+28-4)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 0cd86dc54b0ab..93e2e862deb8a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9818,15 +9818,36 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
       ((getLangOpts().C23 && RHS.get()->getType()->isNullPtrType()) ||
        RHS.get()->isNullPointerConstant(Context,
                                         Expr::NPC_ValueDependentIsNull))) {
+    AssignConvertType Ret = Compatible;
     if (Diagnose || ConvertRHS) {
       CastKind Kind;
       CXXCastPath Path;
       CheckPointerConversion(RHS.get(), LHSType, Kind, Path,
                              /*IgnoreBaseAccess=*/false, Diagnose);
+
+      // If there is a conversion of some kind, check to see what kind of
+      // pointer conversion happened so we can diagnose a C++ compatibility
+      // diagnostic if the conversion is invalid. This only matters if the RHS
+      // is some kind of void pointer.
+      if (Kind != CK_NoOp && !getLangOpts().CPlusPlus) {
+        QualType CanRHS =
+            RHS.get()->getType().getCanonicalType().getUnqualifiedType();
+        QualType CanLHS = LHSType.getCanonicalType().getUnqualifiedType();
+        if (CanRHS->isVoidPointerType() && CanLHS->isPointerType()) {
+          Ret = checkPointerTypesForAssignment(*this, CanLHS, CanRHS,
+                                               RHS.get()->getExprLoc());
+          // Anything that's not considered perfectly compatible would be
+          // incompatible in C++.
+          if (Ret != Compatible)
+            Ret = CompatibleVoidPtrToNonVoidPtr;
+        }
+      }
+
+
       if (ConvertRHS)
         RHS = ImpCastExprToType(RHS.get(), LHSType, Kind, VK_PRValue, &Path);
     }
-    return Compatible;
+    return Ret;
   }
   // C23 6.5.16.1p1: the left operand has type atomic, qualified, or
   // unqualified bool, and the right operand is a pointer or its type is
@@ -13946,10 +13967,8 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
           LHSType->isObjCObjectPointerType())))
       ConvTy = Compatible;
 
-    if (ConvTy == Compatible &&
-        LHSType->isObjCObjectType())
-        Diag(Loc, diag::err_objc_object_assignment)
-          << LHSType;
+    if (IsAssignConvertCompatible(ConvTy) && LHSType->isObjCObjectType())
+      Diag(Loc, diag::err_objc_object_assignment) << LHSType;
 
     // If the RHS is a unary plus or minus, check to see if they = and + are
     // right next to each other.  If so, the user may have typo'd "x =+ 4"
@@ -13971,7 +13990,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
       }
     }
 
-    if (ConvTy == Compatible) {
+    if (IsAssignConvertCompatible(ConvTy)) {
       if (LHSType.getObjCLifetime() == Qualifiers::OCL_Strong) {
         // Warn about retain cycles where a block captures the LHS, but
         // not if the LHS is a simple variable into which the block is
diff --git a/clang/test/Sema/implicit-void-ptr-cast.c b/clang/test/Sema/implicit-void-ptr-cast.c
index df18eeebd9347..a469c49c36b49 100644
--- a/clang/test/Sema/implicit-void-ptr-cast.c
+++ b/clang/test/Sema/implicit-void-ptr-cast.c
@@ -1,8 +1,8 @@
-// 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 -std=c23 -verify=c -Wimplicit-void-ptr-cast %s
+// RUN: %clang_cc1 -fsyntax-only -std=c23 -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
+// RUN: %clang_cc1 -fsyntax-only -std=c23 -verify=good %s
+// RUN: %clang_cc1 -fsyntax-only -std=c23 -verify=good -Wc++-compat -Wno-implicit-void-ptr-cast %s
 // good-no-diagnostics
 
 typedef __typeof__(sizeof(int)) size_t;
@@ -36,3 +36,27 @@ 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 *'}}
 }
+
+void more(void) {
+  __attribute__((address_space(0))) char *b1 = (void *)0; // c-warning {{implicit conversion when initializing '__attribute__((address_space(0))) char *' with an expression of type 'void *' is not permitted in C++}} \
+                                                             cxx-error {{cannot initialize a variable of type '__attribute__((address_space(0))) char *' with an rvalue of type 'void *'}}
+  __attribute__((address_space(0))) void *b2 = (void *)0; // c-warning {{implicit conversion when initializing '__attribute__((address_space(0))) void *' with an expression of type 'void *' is not permitted in C++}} \
+                                                             cxx-error {{cannot initialize a variable of type '__attribute__((address_space(0))) void *' with an rvalue of type 'void *'}}
+  char *b3 = (void *)0; // c-warning {{implicit conversion when initializing 'char *' with an expression of type 'void *' is not permitted in C++}} \
+                           cxx-error {{cannot initialize a variable of type 'char *' with an rvalue of type 'void *'}}
+
+  b1 = (void*)0; // c-warning {{implicit conversion when assigning to '__attribute__((address_space(0))) char *' from type 'void *' is not permitted in C++}} \
+                    cxx-error {{assigning 'void *' to '__attribute__((address_space(0))) char *' changes address space of pointer}}
+
+  b2 = (void*)0; // c-warning {{implicit conversion when assigning to '__attribute__((address_space(0))) void *' from type 'void *' is not permitted in C++}} \
+                    cxx-error {{assigning 'void *' to '__attribute__((address_space(0))) void *' changes address space of pointer}}
+  b2 = (__attribute__((address_space(0))) void *)0;
+  b2 = nullptr;
+  b2 = 0;
+
+  b3 = (void*)0; // c-warning {{implicit conversion when assigning to 'char *' from type 'void *' is not permitted in C++}} \
+                    cxx-error {{assigning to 'char *' from incompatible type 'void *'}}
+  b3 = (char *)0;
+  b3 = nullptr;
+  b3 = 0;
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[alexfh]

Just to confirm: is it intended that char *s = NULL; now generates this warning? I mean, the C library may define NULL as ((void*)0), and char *s = ((void*)0); is clearly not C++-compatible. However, when compiling this code in C++, NULL may expand to something that IS actually okay in C++ (like 0, nullptr or __null). So this warning may be a bit too noisy overall?

https://gcc.godbolt.org/z/hnMfsPjsa

[AaronBallman]

Just to confirm: is it intended that char *s = NULL; now generates this warning? I mean, the C library may define NULL as ((void*)0), and char *s = ((void*)0); is clearly not C++-compatible. However, when compiling this code in C++, NULL may expand to something that IS actually okay in C++ (like 0, nullptr or __null). So this warning may be a bit too noisy overall?

https://gcc.godbolt.org/z/hnMfsPjsa

Hmmm, semi-intended. It's intended to diagnose void foo(void *ptr) { char *s = ptr; }, but a null pointer constant initialization, regardless of form (nullptr, (void *)0, __null, etc) should not be diagnosed. So I think I need to be more clever here; I'll work on a fix (thought it likely won't be until tomorrow at this point). Thank you for bringing this up!

[AaronBallman]

Just to confirm: is it intended that char *s = NULL; now generates this warning? I mean, the C library may define NULL as ((void*)0), and char *s = ((void*)0); is clearly not C++-compatible. However, when compiling this code in C++, NULL may expand to something that IS actually okay in C++ (like 0, nullptr or __null). So this warning may be a bit too noisy overall?
https://gcc.godbolt.org/z/hnMfsPjsa

Hmmm, semi-intended. It's intended to diagnose void foo(void *ptr) { char *s = ptr; }, but a null pointer constant initialization, regardless of form (nullptr, (void *)0, __null, etc) should not be diagnosed. So I think I need to be more clever here; I'll work on a fix (thought it likely won't be until tomorrow at this point). Thank you for bringing this up!

#140724 should address this concern