Update with warning message for comparison to NULL pointer

[Krishna-13-cyber]

Summary

This patch solves the issue pointed out in https://github.com/llvm/llvm-project/issues/42992.
While checking the testcases it fails in one of the cases, I am unsure of the fix of that while rest all seems fine.

[Krishna-13-cyber]

• Apologies for uploading some other/incorrect diff
• Updated Diff

[tbaeder]

Both test failures look like you just need to add the additional new diagnostic.

[Krishna-13-cyber]

• Update with diagnostic addition

[Krishna-13-cyber]

• Update with addition of testcase
• There is an issue for which the build fails, actually when I replace the diagnostic in /clang/test/Sema/conditional-expr.c Line 89, it tells C99 forbids conditional expressions with only one void side and if I remove it shows seen but not expected and vice versa.It regresses either ways.

In the same manner for comparison of address of 'g' equal to a null pointer is always false.

[shafik]

clang/test/Sema/warn-tautological-compare.c

102 ↗

(On Diff #516397)

Please add back the missing newline

clang/test/SemaCXX/constant-expression-cxx2a.cpp

252 ↗

(On Diff #516397)

I don't believe we should be emitting a diagnostic for this case. The static_assert passes. CC @aaron.ballman

[Krishna-13-cyber]

• Updated with reviewed changes

[aaron.ballman]

clang/test/Sema/conditional-expr.c

89 ↗

(On Diff #516857)

The C99 warning should not be dropped -- that was a valid pedantic diagnostic: https://godbolt.org/z/Tz3zGY8d4

clang/test/SemaCXX/constant-expression-cxx2a.cpp

252 ↗

(On Diff #516397)

Agreed, this new diagnostic should not be triggered

[Krishna-13-cyber]

• Updated with re-adding the required diagnostic message

[aaron.ballman]

Precommit CI is still failing on test/SemaCXX/constant-expression-cxx2a.cpp with a false positive warning. You'll need to investigate why that's happening, but I suspect that the warning logic isn't aware that dynamic_cast can return null.

clang/lib/Sema/SemaChecking.cpp

14726 ↗

(On Diff #517151)

clang/test/Sema/warn-tautological-compare.c

97–100 ↗

(On Diff #517151)

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

14726 ↗

(On Diff #517151)

The issue with the failing precommit CI test is because you're ignoring parens and casts with your changes, and to resolve the problem from the issue you should be ignoring parens alone.

Try replacing IgnoreParenCasts() with IgnoreParens() and things should work better.

[Krishna-13-cyber]

• Update with the given suggestion
• Add release notes

Thanks a lot @aaron.ballman for the instant assistance.

[aaron.ballman]

LGTM! There's still a formatting issue in warn-tautological-compare.c but I can fix that up when landing, assuming you still need me to land this on your behalf. (If you can land it on your own, then feel free to fix the formatting issue when you land it.)

[Krishna-13-cyber]

In D149000#4420607, @aaron.ballman wrote:

LGTM! There's still a formatting issue in warn-tautological-compare.c but I can fix that up when landing, assuming you still need me to land this on your behalf. (If you can land it on your own, then feel free to fix the formatting issue when you land it.)

Yes, I don't have the commit access to the repository, It would be great if you could land it on my behalf. Apologies for missing out on the formatting issues.
Thanks a lot!

clang/lib/Sema/SemaDeclCXX.cpp

if (isa<BinaryOperator>(E))
     return false;
 
   return true;
 }
 
 /// Try to print more useful information about a failed static_assert
 /// with expression \E
 void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
-  if (const auto *Op = dyn_cast<BinaryOperator>(E)) {
+  if (const auto *Op = dyn_cast<BinaryOperator>(E);Op && Op->getOpcode() != BO_LOr) {
     const Expr *LHS = Op->getLHS()->IgnoreParenImpCasts();
     const Expr *RHS = Op->getRHS()->IgnoreParenImpCasts();
 
     // Ignore comparisons of boolean expressions with a boolean literal.
     if ((isa<CXXBoolLiteralExpr>(LHS) && RHS->getType()->isBooleanType()) ||
         (isa<CXXBoolLiteralExpr>(RHS) && LHS->getType()->isBooleanType()))
       return;