diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8428,7 +8428,7 @@ def warn_stringcompare : Warning< "result of comparison against %select{a string literal|@encode}0 is " - "unspecified (use strncmp instead)">, + "unspecified (use an explicit string comparison function instead)">, InGroup; def warn_identity_field_assign : Warning< diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10338,7 +10338,6 @@ QualType RHSType = RHS->getType(); if (LHSType->hasFloatingRepresentation() || (LHSType->isBlockPointerType() && !BinaryOperator::isEqualityOp(Opc)) || - LHS->getBeginLoc().isMacroID() || RHS->getBeginLoc().isMacroID() || S.inTemplateInstantiation()) return; @@ -10366,45 +10365,51 @@ AlwaysEqual, // std::strong_ordering::equal from operator<=> }; - if (Expr::isSameComparisonOperand(LHS, RHS)) { - unsigned Result; - switch (Opc) { - case BO_EQ: case BO_LE: case BO_GE: - Result = AlwaysTrue; - break; - case BO_NE: case BO_LT: case BO_GT: - Result = AlwaysFalse; - break; - case BO_Cmp: - Result = AlwaysEqual; - break; - default: - Result = AlwaysConstant; - break; - } - S.DiagRuntimeBehavior(Loc, nullptr, - S.PDiag(diag::warn_comparison_always) - << 0 /*self-comparison*/ - << Result); - } else if (checkForArray(LHSStripped) && checkForArray(RHSStripped)) { - // What is it always going to evaluate to? - unsigned Result; - switch(Opc) { - case BO_EQ: // e.g. array1 == array2 - Result = AlwaysFalse; - break; - case BO_NE: // e.g. array1 != array2 - Result = AlwaysTrue; - break; - default: // e.g. array1 <= array2 - // The best we can say is 'a constant' - Result = AlwaysConstant; - break; + if (!LHS->getBeginLoc().isMacroID() && !RHS->getBeginLoc().isMacroID()) { + if (Expr::isSameComparisonOperand(LHS, RHS)) { + unsigned Result; + switch (Opc) { + case BO_EQ: + case BO_LE: + case BO_GE: + Result = AlwaysTrue; + break; + case BO_NE: + case BO_LT: + case BO_GT: + Result = AlwaysFalse; + break; + case BO_Cmp: + Result = AlwaysEqual; + break; + default: + Result = AlwaysConstant; + break; + } + S.DiagRuntimeBehavior(Loc, nullptr, + S.PDiag(diag::warn_comparison_always) + << 0 /*self-comparison*/ + << Result); + } else if (checkForArray(LHSStripped) && checkForArray(RHSStripped)) { + // What is it always going to evaluate to? + unsigned Result; + switch (Opc) { + case BO_EQ: // e.g. array1 == array2 + Result = AlwaysFalse; + break; + case BO_NE: // e.g. array1 != array2 + Result = AlwaysTrue; + break; + default: // e.g. array1 <= array2 + // The best we can say is 'a constant' + Result = AlwaysConstant; + break; + } + S.DiagRuntimeBehavior(Loc, nullptr, + S.PDiag(diag::warn_comparison_always) + << 1 /*array comparison*/ + << Result); } - S.DiagRuntimeBehavior(Loc, nullptr, - S.PDiag(diag::warn_comparison_always) - << 1 /*array comparison*/ - << Result); } if (isa(LHSStripped)) @@ -10413,7 +10418,7 @@ RHSStripped = RHSStripped->IgnoreParenCasts(); // Warn about comparisons against a string constant (unless the other - // operand is null); the user probably wants strcmp. + // operand is null); the user probably wants string comparison function. Expr *LiteralString = nullptr; Expr *LiteralStringStripped = nullptr; if ((isa(LHSStripped) || isa(LHSStripped)) && diff --git a/clang/test/Sema/exprs.c b/clang/test/Sema/exprs.c --- a/clang/test/Sema/exprs.c +++ b/clang/test/Sema/exprs.c @@ -119,7 +119,7 @@ // PR3753 int test12(const char *X) { - return X == "foo"; // expected-warning {{comparison against a string literal is unspecified (use strncmp instead)}} + return X == "foo"; // expected-warning {{comparison against a string literal is unspecified (use an explicit string comparison function instead)}} } int test12b(const char *X) { diff --git a/clang/test/Sema/warn-stringcompare.c b/clang/test/Sema/warn-stringcompare.c new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-stringcompare.c @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s + +#define DELIM "/" +#define DOT "." +#define NULL (void *)0 + +void test(const char *d) { + if ("/" != d) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; + if (d == "/") // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; + if ("/" != NULL) + return; + if (NULL == "/") + return; + if ("/" != DELIM) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; + if (DELIM == "/") // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; + if (DELIM != NULL) + return; + if (NULL == DELIM) + return; + if (DOT != DELIM) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; + if (DELIM == DOT) // expected-warning {{result of comparison against a string literal is unspecified (use an explicit string comparison function instead)}} + return; +}