xbolva00 updated this revision to Diff 218258.
xbolva00 added a comment.
Fixed review comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66397/new/
https://reviews.llvm.org/D66397
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn-xor-as-pow.cpp
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
}
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<BinaryOperator>(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<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();
+ 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<IntegerLiteral>(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: 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<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"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
