[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-21 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman closed 
https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Alexander Kornienko via cfe-commits

https://github.com/alexfh approved this pull request.

Thanks for the prompt fix! Looks good!

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Aaron Ballman via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

AaronBallman wrote:

Ah, nope! We've got a test for that:
```
 #define SOMETHING_THAT_IS_NOT_NULL (void *)12

  char *ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}

  ...

  ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}
```

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Erich Keane via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

erichkeane wrote:

OH! I  misunderstood what was happening here/missed the call to 
`isNullPointerConstant` above.  I thought this was forgiving ALL `char* F = 
MY_MAGIC_MACRO`'s.

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Erich Keane via cfe-commits

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Aaron Ballman via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

AaronBallman wrote:

That's kind of the point -- this code is specific to the case of a void pointer 
conversion from a null pointer constant.

So we do get the pattern, pass that predicate and get to the point of saying 
"cool, time to diagnose this implicit void * cast" and now we're saying "oh, 
but the RHS is a macro so nevermind, just be quiet about it because the macro 
may expand differently in C++ than it did in C".

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Erich Keane via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

erichkeane wrote:

MOST of those would work with the `isNullPointerConstant` I would think, we get 
the `(void*)0` pattern with that, no?If not, it absolutely should...


https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Erich Keane via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

erichkeane wrote:

This is pretty liberal, right?  Why EVERYTHING? 

Why not just use the common patterns of NULL that we care about?  There is a 
function somewhere to test that it is the 'C" style null-pointer-cast 
(`Expr::isNullPointerConstant`) that does exactly what you want I think.

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Aaron Ballman via cfe-commits


@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {

AaronBallman wrote:

The predicate already checks that:
```
RHS.get()->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull)))
```
so this allows for users who define their own null macros (which does happen): 
https://sourcegraph.com/search?q=context:global+%23define%5B+%5Ct%5D%2B%5BA-Za-z0-9_%5D%2B+%5B%5C%28%5D%3F%5C%28void%5B+%5Ct%5D*%5C*%5C%290%5B%5C%29%5D%3F&patternType=regexp&case=yes&sm=0

https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)


Changes

This silences the diagnostic when the right-hand side is a null pointer 
constant that comes from a macro expansion, such as NULL. However, we do not 
limit to just NULL because other custom macros may expand to an implicit void * 
cast in C while expanding to something else in C++.

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaExpr.cpp (+7-2) 
- (modified) clang/test/Sema/implicit-void-ptr-cast.c (+22) 


``diff
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b18e83b605e4f..6b04f8150b0a1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {
 QualType CanRHS =
 RHS.get()->getType().getCanonicalType().getUnqualifiedType();
 QualType CanLHS = LHSType.getCanonicalType().getUnqualifiedType();
diff --git a/clang/test/Sema/implicit-void-ptr-cast.c 
b/clang/test/Sema/implicit-void-ptr-cast.c
index a469c49c36b49..3c3e153d1dbda 100644
--- a/clang/test/Sema/implicit-void-ptr-cast.c
+++ b/clang/test/Sema/implicit-void-ptr-cast.c
@@ -59,4 +59,26 @@ void more(void) {
   b3 = (char *)0;
   b3 = nullptr;
   b3 = 0;
+
+  // Note that we explicitly silence the diagnostic if the RHS is from a macro
+  // expansion. This allows for things like NULL expanding to different token
+  // sequences depending on language mode, but applies to any macro that
+  // expands to a valid null pointer constant.
+#if defined(__cplusplus)
+  #define NULL 0
+#else
+  #define NULL ((void *)0)
+#endif
+  #define SOMETHING_NOT_SPELLED_NULL nullptr
+  #define SOMETHING_THAT_IS_NOT_NULL (void *)12
+
+  char *ptr1 = NULL; // Ok
+  char *ptr2 = SOMETHING_NOT_SPELLED_NULL; // Ok
+  char *ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}
+
+  ptr1 = NULL; // Ok
+  ptr2 = SOMETHING_NOT_SPELLED_NULL; // Ok
+  ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}
 }

``




https://github.com/llvm/llvm-project/pull/140724
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Don't diagnose null pointer macros in -Wimplicit-void-ptr-cast (PR #140724)

2025-05-20 Thread Aaron Ballman via cfe-commits

https://github.com/AaronBallman created 
https://github.com/llvm/llvm-project/pull/140724

This silences the diagnostic when the right-hand side is a null pointer 
constant that comes from a macro expansion, such as NULL. However, we do not 
limit to just NULL because other custom macros may expand to an implicit void * 
cast in C while expanding to something else in C++.

>From a5e8f77597ecc8ae56f947de5ef236d88e1581d1 Mon Sep 17 00:00:00 2001
From: Aaron Ballman 
Date: Tue, 20 May 2025 08:49:33 -0400
Subject: [PATCH] [C] Don't diagnose null pointer macros in
 -Wimplicit-void-ptr-cast

This silences the diagnostic when the right-hand side is a null pointer
constant that comes from a macro expansion, such as NULL. However, we
do not limit to just NULL because other custom macros may expand to an
implicit void * cast in C while expanding to something else in C++.
---
 clang/lib/Sema/SemaExpr.cpp  |  9 +++--
 clang/test/Sema/implicit-void-ptr-cast.c | 22 ++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b18e83b605e4f..6b04f8150b0a1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9966,8 +9966,13 @@ AssignConvertType 
Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   // 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) {
+  // is some kind of void pointer. We have a carve-out when the RHS is from
+  // a macro expansion because the use of a macro may indicate different
+  // code between C and C++. Consider: char *s = NULL; where NULL is
+  // defined as (void *)0 in C (which would be invalid in C++), but 0 in
+  // C++, which is valid in C++.
+  if (Kind != CK_NoOp && !getLangOpts().CPlusPlus &&
+  !RHS.get()->getBeginLoc().isMacroID()) {
 QualType CanRHS =
 RHS.get()->getType().getCanonicalType().getUnqualifiedType();
 QualType CanLHS = LHSType.getCanonicalType().getUnqualifiedType();
diff --git a/clang/test/Sema/implicit-void-ptr-cast.c 
b/clang/test/Sema/implicit-void-ptr-cast.c
index a469c49c36b49..3c3e153d1dbda 100644
--- a/clang/test/Sema/implicit-void-ptr-cast.c
+++ b/clang/test/Sema/implicit-void-ptr-cast.c
@@ -59,4 +59,26 @@ void more(void) {
   b3 = (char *)0;
   b3 = nullptr;
   b3 = 0;
+
+  // Note that we explicitly silence the diagnostic if the RHS is from a macro
+  // expansion. This allows for things like NULL expanding to different token
+  // sequences depending on language mode, but applies to any macro that
+  // expands to a valid null pointer constant.
+#if defined(__cplusplus)
+  #define NULL 0
+#else
+  #define NULL ((void *)0)
+#endif
+  #define SOMETHING_NOT_SPELLED_NULL nullptr
+  #define SOMETHING_THAT_IS_NOT_NULL (void *)12
+
+  char *ptr1 = NULL; // Ok
+  char *ptr2 = SOMETHING_NOT_SPELLED_NULL; // Ok
+  char *ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}
+
+  ptr1 = NULL; // Ok
+  ptr2 = SOMETHING_NOT_SPELLED_NULL; // Ok
+  ptr3 = SOMETHING_THAT_IS_NOT_NULL; // 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 *'}}
 }

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits