Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -3361,7 +3361,7 @@ "result of '%0' is %1; did you mean '%2'?">, InGroup; 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: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -48,6 +48,12 @@ using namespace clang; using namespace sema; +static void diagnoseXorMisusedAsPow(Sema &S, const ExprResult &XorLHS, + const ExprResult &XorRHS, + const SourceLocation Loc, + const Expr *SubLHS = nullptr, + const Expr *SubRHS = nullptr); + /// Determine whether the use of this declaration is valid, without /// emitting diagnostics. bool Sema::CanUseDecl(NamedDecl *D, bool TreatUnavailableAsInvalid) { @@ -9606,6 +9612,11 @@ if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); + const auto *BO = dyn_cast(LHS.get()->IgnoreParenImpCasts()); + 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). @@ -11081,33 +11092,51 @@ return GetSignedVectorType(vType); } -static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS, - SourceLocation Loc) { +// Nonnull SubLHS and SubRHS expressions represent the sides of the SUB +// expression (X ^ Y) - Z, where (X ^ Y) is SubLHS and Z is SubRHS. +static void diagnoseXorMisusedAsPow(Sema &S, const ExprResult &XorLHS, + const ExprResult &XorRHS, + const SourceLocation Loc, + const Expr *SubLHS, const Expr *SubRHS) { // 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()); + bool ExplicitPlus = 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(RHS.get())) { - if (UO->getOpcode() != UO_Minus) + if (const auto *UO = dyn_cast(XorRHS.get())) { + UnaryOperatorKind Opc = UO->getOpcode(); + if (Opc != UO_Minus && Opc != UO_Plus) return; RHSInt = dyn_cast(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(); + const llvm::APInt &RightSideValue = RHSInt->getValue(); + const llvm::APInt XorValue = LeftSideValue ^ RightSideValue; + if (LeftSideValue.getBitWidth() != RightSideValue.getBitWidth()) + return; + + if (LeftSideValue != 2 && LeftSideValue != 10) + return; + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1, which + // is an incorrect expression to get the maximum value of ULLONG. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) return; CharSourceRange ExprRange = CharSourceRange::getCharRange( @@ -11123,10 +11152,6 @@ 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()); @@ -11138,32 +11163,55 @@ if (Negative) { RightSideIntValue = -RightSideIntValue; 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"); 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) + 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 + } 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" + : "~0ULL"; + 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 @@ -11172,13 +11220,13 @@ 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 +11273,6 @@ 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 +11281,9 @@ 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: 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) { @@ -32,25 +34,31 @@ 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,31 +77,56 @@ 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) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean '~0ULL'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"~0ULL" + // expected-note@-2 {{replace expression with '0x2 ^ 64' or use 'xor' instead of '^' 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' or use 'xor' instead of '^' 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' or use 'xor' instead of '^' to silence this warning}} + res = (2 ^ 64) - 1u; // expected-warning {{result of '2 ^ 64' 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 ^ 64' or use 'xor' instead of '^' 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' or use 'xor' instead of '^' to silence this warning}} 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 xor 10; #ifdef __cplusplus @@ -101,5 +134,14 @@ 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 }