Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -9422,6 +9422,133 @@ << RHSExpr->getSourceRange(); } +static void diagnoseXorMisusedAsPow(Sema &S, const ExprResult &XorLHS, + const ExprResult &XorRHS, + const SourceLocation Loc, + const Expr *SubLHS = nullptr, + const Expr *SubRHS = nullptr) { + bool Negative = false; + const auto *LHSInt = dyn_cast(XorLHS.get()); + const auto *RHSInt = dyn_cast(XorRHS.get()); + + if (!LHSInt) + return; + if (!RHSInt) { + // Check negative literals. + if (const auto *UO = dyn_cast(XorRHS.get())) { + if (UO->getOpcode() != UO_Minus) + return; + RHSInt = dyn_cast(UO->getSubExpr()); + if (!RHSInt) + return; + Negative = true; + } else { + return; + } + } + + if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth()) + return; + + CharSourceRange ExprRange = CharSourceRange::getCharRange( + LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation())); + std::string ExprStr = + Lexer::getSourceText(ExprRange, S.getSourceManager(), S.getLangOpts()); + + CharSourceRange XorRange = + CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc)); + std::string XorStr = + Lexer::getSourceText(XorRange, S.getSourceManager(), S.getLangOpts()); + // Do not diagnose if xor keyword/macro is used. + 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()); + std::string RHSStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(RHSInt->getSourceRange()), + S.getSourceManager(), S.getLangOpts()); + if (LHSStr.empty()) + LHSStr = LeftSideValue.toString(10, false); + if (RHSStr.empty()) + RHSStr = RightSideValue.toString(10, false); + if (ExprStr.empty()) + ExprStr = LHSStr + " ^ " + RHSStr; + + int64_t RightSideIntValue = RightSideValue.getSExtValue(); + if (Negative) { + RightSideIntValue = -RightSideIntValue; + RHSStr = "-" + RHSStr; + } + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideIntValue != 64)) + return; + + StringRef LHSStrRef = LHSStr; + StringRef RHSStrRef = RHSStr; + // Do not diagnose 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"))) + return; + + if (LeftSideValue == 2 && RightSideIntValue >= 0) { + std::string SuggestedExpr = "1 << " + RHSStr; + bool Overflow = false; + llvm::APInt One = (LeftSideValue - 1); + llvm::APInt PowValue = One.sshl_ov(RightSideValue, Overflow); + if (Overflow) { + if (RightSideIntValue < 64) { + S.Diag(Loc, diag::warn_xor_used_as_pow_base) + << ExprStr << XorValue.toString(10, true) << ("1LL << " + RHSStr) + << FixItHint::CreateReplacement(ExprRange, "1LL << " + RHSStr); + } else if (SubLHS && SubRHS && RightSideIntValue == 64) { + const auto *SubInt = dyn_cast(SubRHS); + // Diagnose special case (2 ^ 64) - 1. + if (SubInt && SubInt->getValue() == 1) { + SuggestedExpr = S.getPreprocessor().isMacroDefined("ULLONG_MAX") + ? "ULLONG_MAX" + : "-1LL"; + ExprRange = CharSourceRange::getCharRange( + SubLHS->getBeginLoc(), + S.getLocForEndOfToken(SubInt->getLocation())); + S.Diag(Loc, diag::warn_xor_used_as_pow_base) + << ExprStr << XorValue.toString(10, true) << SuggestedExpr + << PowValue.toString(10, true) + << FixItHint::CreateReplacement(ExprRange, SuggestedExpr); + } else { + return; + } + } else { + return; + } + } else { + S.Diag(Loc, diag::warn_xor_used_as_pow_base_extra) + << ExprStr << XorValue.toString(10, true) << SuggestedExpr + << PowValue.toString(10, true) + << FixItHint::CreateReplacement( + ExprRange, (RightSideIntValue == 0) ? "1" : SuggestedExpr); + } + + S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr); + } 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); + } +} + // C99 6.5.6 QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, BinaryOperatorKind Opc, @@ -9536,6 +9663,11 @@ if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); + const auto *BO = dyn_cast(LHS.get()->IgnoreParens()); + if (BO && BO->getOpcode() == BO_Xor) + diagnoseXorMisusedAsPow(*this, BO->getLHS(), BO->getRHS(), Loc, LHS.get(), + RHS.get()); + // Enforce type constraints: C99 6.5.6p3. // Handle the common case first (both operands are arithmetic). @@ -11011,107 +11143,6 @@ return GetSignedVectorType(vType); } -static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS, - SourceLocation Loc) { - // Do not diagnose macros. - if (Loc.isMacroID()) - return; - - bool Negative = false; - const auto *LHSInt = dyn_cast(LHS.get()); - const auto *RHSInt = dyn_cast(RHS.get()); - - if (!LHSInt) - return; - if (!RHSInt) { - // Check negative literals. - if (const auto *UO = dyn_cast(RHS.get())) { - if (UO->getOpcode() != UO_Minus) - return; - RHSInt = dyn_cast(UO->getSubExpr()); - if (!RHSInt) - return; - Negative = true; - } else { - return; - } - } - - if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth()) - return; - - CharSourceRange ExprRange = CharSourceRange::getCharRange( - LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation())); - llvm::StringRef ExprStr = - Lexer::getSourceText(ExprRange, S.getSourceManager(), S.getLangOpts()); - - CharSourceRange XorRange = - CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc)); - llvm::StringRef XorStr = - Lexer::getSourceText(XorRange, S.getSourceManager(), S.getLangOpts()); - // Do not diagnose if xor keyword/macro is used. - 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()); - std::string RHSStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(RHSInt->getSourceRange()), - S.getSourceManager(), S.getLangOpts()); - - int64_t RightSideIntValue = RightSideValue.getSExtValue(); - if (Negative) { - RightSideIntValue = -RightSideIntValue; - RHSStr = "-" + RHSStr; - } - - StringRef LHSStrRef = LHSStr; - StringRef RHSStrRef = RHSStr; - // Do not diagnose 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"))) - return; - - if (LeftSideValue == 2 && RightSideIntValue >= 0) { - std::string SuggestedExpr = "1 << " + RHSStr; - bool Overflow = false; - llvm::APInt One = (LeftSideValue - 1); - llvm::APInt PowValue = One.sshl_ov(RightSideValue, Overflow); - if (Overflow) { - if (RightSideIntValue < 64) - S.Diag(Loc, diag::warn_xor_used_as_pow_base) - << ExprStr << XorValue.toString(10, true) << ("1LL << " + RHSStr) - << FixItHint::CreateReplacement(ExprRange, "1LL << " + RHSStr); - else - // TODO: 2 ^ 64 - 1 - return; - } else { - S.Diag(Loc, diag::warn_xor_used_as_pow_base_extra) - << ExprStr << XorValue.toString(10, true) << SuggestedExpr - << PowValue.toString(10, true) - << FixItHint::CreateReplacement( - ExprRange, (RightSideIntValue == 0) ? "1" : SuggestedExpr); - } - - S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr); - } 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); - } -} - QualType Sema::CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc) { // Ensure that either both operands are of the same vector type, or Index: test/SemaCXX/warn-xor-as-pow.cpp =================================================================== --- test/SemaCXX/warn-xor-as-pow.cpp +++ test/SemaCXX/warn-xor-as-pow.cpp @@ -10,8 +10,10 @@ #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 flexor 7 @@ -21,7 +23,7 @@ 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) { @@ -51,6 +53,9 @@ 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}} + 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' to silence this warning}} res = 0x2 ^ 16; res = 2 xor 16; @@ -63,16 +68,35 @@ res = 0b10 ^ 16; res = 0B10 ^ 16; res = 2 ^ 0b100; - res = XOR(2, 16); + res = XOR(2, 16); // expected-warning {{result of '2 ^ 16' is 18; did you mean '1 << 16' (65536)?}} + // expected-note@-1 {{replace expression with '0x2 ^ 16' to silence this warning}} unsigned char two = 2; res = two ^ 16; 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; + res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean '-1LL'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"-1LL" + // expected-note@-2 {{replace expression with '0x2 ^ 64' to silence this warning}} +#define ULLONG_MAX 18446744073709551615ULL + res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean 'ULLONG_MAX'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"ULLONG_MAX" + // expected-note@-2 {{replace expression with '0x2 ^ 64' to silence this warning}} + res = (2 ^ 64) - 2; + res = (2 ^ IOP) - 1; // expected-warning {{result of '2 ^ IOP' is 66; did you mean 'ULLONG_MAX'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:22}:"ULLONG_MAX" + // expected-note@-2 {{replace expression with '0x2 ^ IOP' to silence this warning}} + res = (2 ^ 65) - 1; + res = (2 + 64) - 1; + res = (3 ^ 64) - 1; + res = (2 ^ 8) - 1; // expected-warning {{result of '2 ^ 8' is 10; did you mean '1 << 8' (256)?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"1 << 8" + // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}} - res = EPSILON; + res = EPSILON; // expected-warning {{result of 'EPSILON' is 294; did you mean '1e-300'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e-300" + // expected-note@-2 {{replace expression with '0xA ^ -300' to silence this warning}} 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}} @@ -91,6 +115,9 @@ 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}} + 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' 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}}