]> granicus.if.org Git - clang/commitdiff
[Diagnostics] Minor improvements for -Wxor-used-as-pow
authorDavid Bolvansky <david.bolvansky@gmail.com>
Thu, 5 Sep 2019 20:50:48 +0000 (20:50 +0000)
committerDavid Bolvansky <david.bolvansky@gmail.com>
Thu, 5 Sep 2019 20:50:48 +0000 (20:50 +0000)
Extracted from D66397; implemented suggestion for 2^64; tests revisited.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371122 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn-xor-as-pow.cpp

index 278bcae3dc903d6819e99006165a18a54ef640bf..00d05f971b9d7db73aa10ecfdbe09270fa0cc00d 100644 (file)
@@ -3354,6 +3354,9 @@ def warn_address_of_reference_bool_conversion : Warning<
   "code; pointer may be assumed to always convert to true">,
   InGroup<UndefinedBoolConversion>;
 
+def warn_xor_used_as_pow : Warning<
+  "result of '%0' is %1; did you mean an exponentiation?">,
+  InGroup<XorUsedAsPow>;
 def warn_xor_used_as_pow_base_extra : Warning<
   "result of '%0' is %1; did you mean '%2' (%3)?">,
   InGroup<XorUsedAsPow>;
@@ -3361,7 +3364,7 @@ def warn_xor_used_as_pow_base : Warning<
   "result of '%0' is %1; did you mean '%2'?">,
   InGroup<XorUsedAsPow>;
 def note_xor_used_as_pow_silence : Note<
-  "replace expression with '%0' to silence this warning">;
+  "replace expression with '%0' %select{|or use 'xor' instead of '^' }1to silence this warning">;
 
 def warn_null_pointer_compare : Warning<
     "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
index 5f5f4c8d0446c4b4b2023259d8fe76e1c2af6067..b5ad208e998aa4564bbfba18c35c3537566d2fb6 100644 (file)
@@ -11081,33 +11081,42 @@ QualType Sema::CheckVectorCompareOperands(ExprResult &LHS, ExprResult &RHS,
   return GetSignedVectorType(vType);
 }
 
-static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
-                                    SourceLocation Loc) {
+static void diagnoseXorMisusedAsPow(Sema &S, const ExprResult &XorLHS,
+                                    const ExprResult &XorRHS,
+                                    const SourceLocation Loc) {
   // Do not diagnose macros.
   if (Loc.isMacroID())
     return;
 
   bool Negative = false;
-  const auto *LHSInt = dyn_cast<IntegerLiteral>(LHS.get());
-  const auto *RHSInt = dyn_cast<IntegerLiteral>(RHS.get());
+  bool ExplicitPlus = false;
+  const auto *LHSInt = dyn_cast<IntegerLiteral>(XorLHS.get());
+  const auto *RHSInt = dyn_cast<IntegerLiteral>(XorRHS.get());
 
   if (!LHSInt)
     return;
   if (!RHSInt) {
     // Check negative literals.
-    if (const auto *UO = dyn_cast<UnaryOperator>(RHS.get())) {
-      if (UO->getOpcode() != UO_Minus)
+    if (const auto *UO = dyn_cast<UnaryOperator>(XorRHS.get())) {
+      UnaryOperatorKind Opc = UO->getOpcode();
+      if (Opc != UO_Minus && Opc != UO_Plus)
         return;
       RHSInt = dyn_cast<IntegerLiteral>(UO->getSubExpr());
       if (!RHSInt)
         return;
-      Negative = true;
+      Negative = (Opc == UO_Minus);
+      ExplicitPlus = !Negative;
     } else {
       return;
     }
   }
 
-  if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth())
+  const llvm::APInt &LeftSideValue = LHSInt->getValue();
+  llvm::APInt RightSideValue = RHSInt->getValue();
+  if (LeftSideValue != 2 && LeftSideValue != 10)
+    return;
+
+  if (LeftSideValue.getBitWidth() != RightSideValue.getBitWidth())
     return;
 
   CharSourceRange ExprRange = CharSourceRange::getCharRange(
@@ -11123,10 +11132,6 @@ static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
   if (XorStr == "xor")
     return;
 
-  const llvm::APInt &LeftSideValue = LHSInt->getValue();
-  const llvm::APInt &RightSideValue = RHSInt->getValue();
-  const llvm::APInt XorValue = LeftSideValue ^ RightSideValue;
-
   std::string LHSStr = Lexer::getSourceText(
       CharSourceRange::getTokenRange(LHSInt->getSourceRange()),
       S.getSourceManager(), S.getLangOpts());
@@ -11134,23 +11139,30 @@ static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
       CharSourceRange::getTokenRange(RHSInt->getSourceRange()),
       S.getSourceManager(), S.getLangOpts());
 
-  int64_t RightSideIntValue = RightSideValue.getSExtValue();
   if (Negative) {
-    RightSideIntValue = -RightSideIntValue;
+    RightSideValue = -RightSideValue;
     RHSStr = "-" + RHSStr;
+  } else if (ExplicitPlus) {
+    RHSStr = "+" + RHSStr;
   }
 
   StringRef LHSStrRef = LHSStr;
   StringRef RHSStrRef = RHSStr;
-  // Do not diagnose binary, hexadecimal, octal literals.
+  // Do not diagnose literals with digit separators, binary, hexadecimal, octal
+  // literals.
   if (LHSStrRef.startswith("0b") || LHSStrRef.startswith("0B") ||
       RHSStrRef.startswith("0b") || RHSStrRef.startswith("0B") ||
       LHSStrRef.startswith("0x") || LHSStrRef.startswith("0X") ||
       RHSStrRef.startswith("0x") || RHSStrRef.startswith("0X") ||
       (LHSStrRef.size() > 1 && LHSStrRef.startswith("0")) ||
-      (RHSStrRef.size() > 1 && RHSStrRef.startswith("0")))
+      (RHSStrRef.size() > 1 && RHSStrRef.startswith("0")) ||
+      LHSStrRef.find('\'') != StringRef::npos ||
+      RHSStrRef.find('\'') != StringRef::npos)
     return;
 
+  bool SuggestXor = S.getLangOpts().CPlusPlus || S.getPreprocessor().isMacroDefined("xor");
+  const llvm::APInt XorValue = LeftSideValue ^ RightSideValue;
+  int64_t RightSideIntValue = RightSideValue.getSExtValue();
   if (LeftSideValue == 2 && RightSideIntValue >= 0) {
     std::string SuggestedExpr = "1 << " + RHSStr;
     bool Overflow = false;
@@ -11161,8 +11173,9 @@ static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
         S.Diag(Loc, diag::warn_xor_used_as_pow_base)
             << ExprStr << XorValue.toString(10, true) << ("1LL << " + RHSStr)
             << FixItHint::CreateReplacement(ExprRange, "1LL << " + RHSStr);
+      else if (RightSideIntValue == 64)
+        S.Diag(Loc, diag::warn_xor_used_as_pow) << ExprStr << XorValue.toString(10, true);
       else
-         // TODO: 2 ^ 64 - 1
         return;
     } else {
       S.Diag(Loc, diag::warn_xor_used_as_pow_base_extra)
@@ -11172,13 +11185,13 @@ static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
                  ExprRange, (RightSideIntValue == 0) ? "1" : SuggestedExpr);
     }
 
-    S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr);
+    S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr) << SuggestXor;
   } else if (LeftSideValue == 10) {
     std::string SuggestedValue = "1e" + std::to_string(RightSideIntValue);
     S.Diag(Loc, diag::warn_xor_used_as_pow_base)
         << ExprStr << XorValue.toString(10, true) << SuggestedValue
         << FixItHint::CreateReplacement(ExprRange, SuggestedValue);
-    S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0xA ^ " + RHSStr);
+    S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0xA ^ " + RHSStr) << SuggestXor;
   }
 }
 
@@ -11225,9 +11238,6 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
   if (Opc == BO_And)
     diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc);
 
-  if (Opc == BO_Xor)
-    diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
-
   ExprResult LHSResult = LHS, RHSResult = RHS;
   QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
                                                  IsCompAssign);
@@ -11236,6 +11246,9 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
   LHS = LHSResult.get();
   RHS = RHSResult.get();
 
+  if (Opc == BO_Xor)
+    diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
+
   if (!compType.isNull() && compType->isIntegralOrUnscopedEnumerationType())
     return compType;
   return InvalidOperands(Loc, LHS, RHS);
index 8ff32a249e0b0ac641061be0f5789f2dbeea9cd9..e024c288d7f992fa6623fad28b0f8387ac0e4781 100644 (file)
 #define XOR(x, y) (x ^ y)
 #define TWO 2
 #define TEN 10
+#define IOP 64
 #define TWO_ULL 2ULL
 #define EPSILON 10 ^ -300
+#define ALPHA_OFFSET 3
+#define EXP 3
 
 #define flexor 7
 
@@ -21,7 +24,7 @@ constexpr long long operator"" _xor(unsigned long long v) { return v; }
 constexpr long long operator"" _0b(unsigned long long v) { return v; }
 constexpr long long operator"" _0X(unsigned long long v) { return v; }
 #else
-#define xor    ^ // iso646.h
+#define xor ^ // iso646.h
 #endif
 
 void test(unsigned a, unsigned b) {
@@ -32,25 +35,31 @@ void test(unsigned a, unsigned b) {
   res = 2 ^ -1;
   res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2; did you mean '1 << 0' (1)?}}
                // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
-               // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+               // expected-note@-2 {{replace expression with '0x2 ^ 0' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3; did you mean '1 << 1' (2)?}}
                // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 1"
-               // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+               // expected-note@-2 {{replace expression with '0x2 ^ 1' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0; did you mean '1 << 2' (4)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 2"
-  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10; did you mean '1 << 8' (256)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ +8; // expected-warning {{result of '2 ^ +8' is 10; did you mean '1 << +8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << +8"
+  // expected-note@-2 {{replace expression with '0x2 ^ +8' or use 'xor' instead of '^' to silence this warning}}
   res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10; did you mean '1 << 8' (256)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18; did you mean '1 << 16' (65536)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << 16"
-  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you mean '1 << TEN' (1024)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN"
-  // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ TEN' or use 'xor' instead of '^' to silence this warning}}
+  res = res + (2 ^ ALPHA_OFFSET); // expected-warning {{result of '2 ^ ALPHA_OFFSET' is 1; did you mean '1 << ALPHA_OFFSET' (8)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:32}:"1 << ALPHA_OFFSET"
+  // expected-note@-2 {{replace expression with '0x2 ^ ALPHA_OFFSET' or use 'xor' instead of '^' to silence this warning}}
   res = 0x2 ^ 16;
   res = 2 xor 16;
 
@@ -69,37 +78,60 @@ void test(unsigned a, unsigned b) {
   res = TWO_ULL ^ 16;
   res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; did you mean '1LL << 32'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL << 32"
-  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
-  res = 2 ^ 64;
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; did you mean an exponentiation?}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ 65;
 
   res = EPSILON;
   res = 10 ^ 0; // expected-warning {{result of '10 ^ 0' is 10; did you mean '1e0'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e0"
-  // expected-note@-2 {{replace expression with '0xA ^ 0' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 0' or use 'xor' instead of '^' to silence this warning}}
   res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11; did you mean '1e1'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e1"
-  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 1' or use 'xor' instead of '^' to silence this warning}}
   res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8; did you mean '1e2'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e2"
-  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 2' or use 'xor' instead of '^' to silence this warning}}
   res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14; did you mean '1e4'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e4"
-  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 4' or use 'xor' instead of '^' to silence this warning}}
+  res = 10 ^ +4; // expected-warning {{result of '10 ^ +4' is 14; did you mean '1e4'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e4"
+  // expected-note@-2 {{replace expression with '0xA ^ +4' or use 'xor' instead of '^' to silence this warning}}
   res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0; did you mean '1e10'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e10"
-  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 10' or use 'xor' instead of '^' to silence this warning}}
   res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0; did you mean '1e10'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e10"
-  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 10' or use 'xor' instead of '^' to silence this warning}}
+  res = 10 ^ TEN; // expected-warning {{result of '10 ^ TEN' is 0; did you mean '1e10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e10"
+  // expected-note@-2 {{replace expression with '0xA ^ TEN' or use 'xor' instead of '^' to silence this warning}}
   res = 10 ^ 100; // expected-warning {{result of '10 ^ 100' is 110; did you mean '1e100'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e100"
-  // expected-note@-2 {{replace expression with '0xA ^ 100' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0xA ^ 100' or use 'xor' instead of '^' to silence this warning}}
   res = 0xA ^ 10;
+  res = 10 ^ -EXP; // expected-warning {{result of '10 ^ -EXP' is -9; did you mean '1e-3'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:18}:"1e-3"
+  // expected-note@-2 {{replace expression with '0xA ^ -EXP' or use 'xor' instead of '^' to silence this warning}}
+  res = 10 ^ +EXP; // expected-warning {{result of '10 ^ +EXP' is 9; did you mean '1e3'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:18}:"1e3"
+  // expected-note@-2 {{replace expression with '0xA ^ +EXP' or use 'xor' instead of '^' to silence this warning}}
   res = 10 xor 10;
 #ifdef __cplusplus
   res = 10 ^ 5_xor;
   res = 10_xor ^ 5;
   res = 10 ^ 5_0b;
   res = 10_0X ^ 5;
+  res = 2 ^ 2'000;
+  res = 2 ^ 0b0110'1001;
+  res = 10 ^ 2'000;
+#else
+#undef xor
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11; did you mean '1e1'?}}
+  // expected-note@-1 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3; did you mean '1 << 1' (2)?}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 1' to silence this warning}}
 #endif
 }