fix unnecessary warning when using bitand with boolean operators

[11happy]

Overview:
This pull request fixes #77601 where using the bitand operator with boolean operands should not trigger the warning, as it would indicate an intentional use of bitwise AND rather than a typo or error.

Testing:

• Tested the updated code.
• Verified that other functionalities remain unaffected.
  

Dependencies:

• No dependencies on other pull requests.

CC:

• @dtcxzyw

[llvmbot]

@llvm/pr-subscribers-clang

Author: Bhuminjay Soni (11happy)

Changes

Overview:
This pull request fixes #77601 where using the bitand operator with boolean operands should not trigger the warning, as it would indicate an intentional use of bitwise AND rather than a typo or error.

Testing:

• Tested the updated code.
• Verified that other functionalities remain unaffected.
  

Dependencies:

• No dependencies on other pull requests.

CC:

• @dtcxzyw

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaChecking.cpp (+18-6)
• (modified) clang/test/Sema/warn-bitwise-and-bool.c (+2-2)
• (modified) clang/test/Sema/warn-bitwise-or-bool.c (+2-2)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 502b24bcdf8b42..e43892cbf35890 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16191,12 +16191,24 @@ static void AnalyzeImplicitConversions(
         BO->getRHS()->isKnownToHaveBooleanValue() &&
         BO->getLHS()->HasSideEffects(S.Context) &&
         BO->getRHS()->HasSideEffects(S.Context)) {
-      S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
-          << (BO->getOpcode() == BO_And ? "&" : "|") << OrigE->getSourceRange()
-          << FixItHint::CreateReplacement(
-                 BO->getOperatorLoc(),
-                 (BO->getOpcode() == BO_And ? "&&" : "||"));
-      S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
+      clang::SourceManager &SM = S.getSourceManager();
+      clang::LangOptions LO = S.getLangOpts();
+      clang::SourceLocation BLoc = BO->getOperatorLoc();
+      clang::SourceLocation ELoc =
+          clang::Lexer::getLocForEndOfToken(BLoc, 0, SM, LO);
+      llvm::StringRef SR = clang::Lexer::getSourceText(
+          clang::CharSourceRange::getTokenRange(BLoc, ELoc), SM, LO);
+
+      if (SR.str() == "&" || SR.str() == "|") {
+
+        S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
+            << (BO->getOpcode() == BO_And ? "&" : "|")
+            << OrigE->getSourceRange()
+            << FixItHint::CreateReplacement(
+                   BO->getOperatorLoc(),
+                   (BO->getOpcode() == BO_And ? "&&" : "||"));
+        S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
+      }
     }
 
   // For conditional operators, we analyze the arguments as if they
diff --git a/clang/test/Sema/warn-bitwise-and-bool.c b/clang/test/Sema/warn-bitwise-and-bool.c
index 6bec1be1abdef6..23e7afcc59f8aa 100644
--- a/clang/test/Sema/warn-bitwise-and-bool.c
+++ b/clang/test/Sema/warn-bitwise-and-bool.c
@@ -45,8 +45,8 @@ void test(boolean a, boolean b, int *p, volatile int *q, int i) {
   b = bar() & (i > 4);
   b = (i == 7) & foo();
 #ifdef __cplusplus
-  b = foo() bitand bar(); // expected-warning {{use of bitwise '&' with boolean operands}}
-                          // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  b = foo() bitand bar(); // Ok, no warning expected
+                          
 #endif
 
   if (foo() & bar())      // expected-warning {{use of bitwise '&' with boolean operands}}
diff --git a/clang/test/Sema/warn-bitwise-or-bool.c b/clang/test/Sema/warn-bitwise-or-bool.c
index ae86790901aac5..e84f59cf8f766a 100644
--- a/clang/test/Sema/warn-bitwise-or-bool.c
+++ b/clang/test/Sema/warn-bitwise-or-bool.c
@@ -45,8 +45,8 @@ void test(boolean a, boolean b, int *p, volatile int *q, int i) {
   b = bar() | (i > 4);
   b = (i == 7) | foo();
 #ifdef __cplusplus
-  b = foo() bitor bar();  // expected-warning {{use of bitwise '|' with boolean operands}}
-                          // expected-note@-1 {{cast one or both operands to int to silence this warning}}
+  b = foo() bitor bar();  //Ok, no warning expected
+                          
 #endif
 
   if (foo() | bar())      // expected-warning {{use of bitwise '|' with boolean operands}}

[11happy]

Humble Ping!

[AaronBallman]

Thank you for the improvement! Please be sure to add a release note about the changes as well.

[AaronBallman]

You should also add test coverage for C with a macro definition for bitand and bitor, and it'd be good to add a test (both C and C++) that does something like:

#define my_fancy_bit_and &
b = foo() my_fancy_bit_and bar();

to show this intentionally works with user-defined macros as well.

[11happy]

done

[11happy]

currently the macro definition for both & and bitand is not giving any warning however should the & one warn? or current working is correct behaviour as expected?

[AaronBallman]

currently the macro definition for both & and bitand is not giving any warning however should the & one warn? or current working is correct behaviour as expected?

The current behavior is what I'd expect. The idea being: if the user is using & directly, it's not clear whether they accidentally meant to use && instead, but if they use something with an identifier (a macro, whether it's named bitand or not), then there's very little chance they meant && instead.

[AaronBallman]

LGTM with a small tweak to the release notes.