r321972 - Factor out common tautological comparison code from scalar and vector compare checking.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 7 13:57:48 PST 2018
Author: rsmith
Date: Sun Jan 7 13:57:48 2018
New Revision: 321972
URL: http://llvm.org/viewvc/llvm-project?rev=321972&view=rev
Log:
Factor out common tautological comparison code from scalar and vector compare checking.
In passing, improve vector compare diagnostic to match scalar compare diagnostic.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/ext_vector_comparisons.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=321972&r1=321971&r2=321972&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Jan 7 13:57:48 2018
@@ -7906,7 +7906,7 @@ def note_ref_subobject_of_member_declare
// should result in a warning, since these always evaluate to a constant.
// Array comparisons have similar warnings
def warn_comparison_always : Warning<
- "%select{self-|array }0comparison always evaluates to %select{false|true|a constant}1">,
+ "%select{self-|array }0comparison always evaluates to %select{a constant|%2}1">,
InGroup<TautologicalCompare>;
def warn_comparison_bitwise_always : Warning<
"bitwise comparison always evaluates to %select{false|true}0">,
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=321972&r1=321971&r2=321972&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Sun Jan 7 13:57:48 2018
@@ -9580,7 +9580,8 @@ public:
bool AllowBothBool, bool AllowBoolConversion);
QualType GetSignedVectorType(QualType V);
QualType CheckVectorCompareOperands(ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool isRelational);
+ SourceLocation Loc,
+ BinaryOperatorKind Opc);
QualType CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc);
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=321972&r1=321971&r2=321972&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Sun Jan 7 13:57:48 2018
@@ -9587,19 +9587,119 @@ static void diagnoseLogicalNotOnLHSofChe
// Get the decl for a simple expression: a reference to a variable,
// an implicit C++ field reference, or an implicit ObjC ivar reference.
static ValueDecl *getCompareDecl(Expr *E) {
- if (DeclRefExpr* DR = dyn_cast<DeclRefExpr>(E))
+ if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E))
return DR->getDecl();
- if (ObjCIvarRefExpr* Ivar = dyn_cast<ObjCIvarRefExpr>(E)) {
+ if (ObjCIvarRefExpr *Ivar = dyn_cast<ObjCIvarRefExpr>(E)) {
if (Ivar->isFreeIvar())
return Ivar->getDecl();
}
- if (MemberExpr* Mem = dyn_cast<MemberExpr>(E)) {
+ if (MemberExpr *Mem = dyn_cast<MemberExpr>(E)) {
if (Mem->isImplicitAccess())
return Mem->getMemberDecl();
}
return nullptr;
}
+/// Diagnose some forms of syntactically-obvious tautological comparison.
+static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
+ Expr *LHS, Expr *RHS,
+ BinaryOperatorKind Opc) {
+ Expr *LHSStripped = LHS->IgnoreParenImpCasts();
+ Expr *RHSStripped = RHS->IgnoreParenImpCasts();
+
+ QualType LHSType = LHS->getType();
+ QualType RHSType = RHS->getType();
+ if (LHSType->hasFloatingRepresentation() ||
+ (LHSType->isBlockPointerType() && !BinaryOperator::isEqualityOp(Opc)) ||
+ LHS->getLocStart().isMacroID() || RHS->getLocStart().isMacroID() ||
+ S.inTemplateInstantiation())
+ return;
+
+ // For non-floating point types, check for self-comparisons of the form
+ // x == x, x != x, x < x, etc. These always evaluate to a constant, and
+ // often indicate logic errors in the program.
+ //
+ // NOTE: Don't warn about comparison expressions resulting from macro
+ // expansion. Also don't warn about comparisons which are only self
+ // comparisons within a template specialization. The warnings should catch
+ // obvious cases in the definition of the template anyways. The idea is to
+ // warn when the typed comparison operator will always evaluate to the same
+ // result.
+ ValueDecl *DL = getCompareDecl(LHSStripped);
+ ValueDecl *DR = getCompareDecl(RHSStripped);
+ if (DL && DR && DL == DR && !IsWithinTemplateSpecialization(DL)) {
+ StringRef Result;
+ switch (Opc) {
+ case BO_EQ: case BO_LE: case BO_GE:
+ Result = "true";
+ break;
+ case BO_NE: case BO_LT: case BO_GT:
+ Result = "false";
+ break;
+ case BO_Cmp:
+ Result = "'std::strong_ordering::equal'";
+ break;
+ default:
+ break;
+ }
+ S.DiagRuntimeBehavior(Loc, nullptr,
+ S.PDiag(diag::warn_comparison_always)
+ << 0 /*self-comparison*/ << !Result.empty()
+ << Result);
+ } else if (DL && DR && LHSType->isArrayType() && RHSType->isArrayType() &&
+ !DL->getType()->isReferenceType() &&
+ !DR->getType()->isReferenceType()) {
+ // What is it always going to evaluate to?
+ // FIXME: This is wrong if DL and DR are different Decls for the same
+ // entity. It's also wrong if DL and/or DR are weak declarations.
+ StringRef Result;
+ switch(Opc) {
+ case BO_EQ: // e.g. array1 == array2
+ Result = "false";
+ break;
+ case BO_NE: // e.g. array1 != array2
+ Result = "true";
+ break;
+ default: // e.g. array1 <= array2
+ // The best we can say is 'a constant'
+ break;
+ }
+ S.DiagRuntimeBehavior(Loc, nullptr,
+ S.PDiag(diag::warn_comparison_always)
+ << 1 /*array comparison*/
+ << !Result.empty() << Result);
+ }
+
+ if (isa<CastExpr>(LHSStripped))
+ LHSStripped = LHSStripped->IgnoreParenCasts();
+ if (isa<CastExpr>(RHSStripped))
+ RHSStripped = RHSStripped->IgnoreParenCasts();
+
+ // Warn about comparisons against a string constant (unless the other
+ // operand is null); the user probably wants strcmp.
+ Expr *LiteralString = nullptr;
+ Expr *LiteralStringStripped = nullptr;
+ if ((isa<StringLiteral>(LHSStripped) || isa<ObjCEncodeExpr>(LHSStripped)) &&
+ !RHSStripped->isNullPointerConstant(S.Context,
+ Expr::NPC_ValueDependentIsNull)) {
+ LiteralString = LHS;
+ LiteralStringStripped = LHSStripped;
+ } else if ((isa<StringLiteral>(RHSStripped) ||
+ isa<ObjCEncodeExpr>(RHSStripped)) &&
+ !LHSStripped->isNullPointerConstant(S.Context,
+ Expr::NPC_ValueDependentIsNull)) {
+ LiteralString = RHS;
+ LiteralStringStripped = RHSStripped;
+ }
+
+ if (LiteralString) {
+ S.DiagRuntimeBehavior(Loc, nullptr,
+ S.PDiag(diag::warn_stringcompare)
+ << isa<ObjCEncodeExpr>(LiteralStringStripped)
+ << LiteralString->getSourceRange());
+ }
+}
+
// C99 6.5.8, C++ [expr.rel]
QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc, BinaryOperatorKind Opc,
@@ -9609,91 +9709,14 @@ QualType Sema::CheckCompareOperands(Expr
// Handle vector comparisons separately.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType())
- return CheckVectorCompareOperands(LHS, RHS, Loc, IsRelational);
+ return CheckVectorCompareOperands(LHS, RHS, Loc, Opc);
QualType LHSType = LHS.get()->getType();
QualType RHSType = RHS.get()->getType();
- Expr *LHSStripped = LHS.get()->IgnoreParenImpCasts();
- Expr *RHSStripped = RHS.get()->IgnoreParenImpCasts();
-
checkEnumComparison(*this, Loc, LHS.get(), RHS.get());
diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc);
-
- if (!LHSType->hasFloatingRepresentation() &&
- !(LHSType->isBlockPointerType() && IsRelational) &&
- !LHS.get()->getLocStart().isMacroID() &&
- !RHS.get()->getLocStart().isMacroID() &&
- !inTemplateInstantiation()) {
- // For non-floating point types, check for self-comparisons of the form
- // x == x, x != x, x < x, etc. These always evaluate to a constant, and
- // often indicate logic errors in the program.
- //
- // NOTE: Don't warn about comparison expressions resulting from macro
- // expansion. Also don't warn about comparisons which are only self
- // comparisons within a template specialization. The warnings should catch
- // obvious cases in the definition of the template anyways. The idea is to
- // warn when the typed comparison operator will always evaluate to the same
- // result.
- ValueDecl *DL = getCompareDecl(LHSStripped);
- ValueDecl *DR = getCompareDecl(RHSStripped);
- if (DL && DR && DL == DR && !IsWithinTemplateSpecialization(DL)) {
- DiagRuntimeBehavior(Loc, nullptr, PDiag(diag::warn_comparison_always)
- << 0 // self-
- << (Opc == BO_EQ
- || Opc == BO_LE
- || Opc == BO_GE));
- } else if (DL && DR && LHSType->isArrayType() && RHSType->isArrayType() &&
- !DL->getType()->isReferenceType() &&
- !DR->getType()->isReferenceType()) {
- // what is it always going to eval to?
- char always_evals_to;
- switch(Opc) {
- case BO_EQ: // e.g. array1 == array2
- always_evals_to = 0; // false
- break;
- case BO_NE: // e.g. array1 != array2
- always_evals_to = 1; // true
- break;
- default:
- // best we can say is 'a constant'
- always_evals_to = 2; // e.g. array1 <= array2
- break;
- }
- DiagRuntimeBehavior(Loc, nullptr, PDiag(diag::warn_comparison_always)
- << 1 // array
- << always_evals_to);
- }
-
- if (isa<CastExpr>(LHSStripped))
- LHSStripped = LHSStripped->IgnoreParenCasts();
- if (isa<CastExpr>(RHSStripped))
- RHSStripped = RHSStripped->IgnoreParenCasts();
-
- // Warn about comparisons against a string constant (unless the other
- // operand is null), the user probably wants strcmp.
- Expr *literalString = nullptr;
- Expr *literalStringStripped = nullptr;
- if ((isa<StringLiteral>(LHSStripped) || isa<ObjCEncodeExpr>(LHSStripped)) &&
- !RHSStripped->isNullPointerConstant(Context,
- Expr::NPC_ValueDependentIsNull)) {
- literalString = LHS.get();
- literalStringStripped = LHSStripped;
- } else if ((isa<StringLiteral>(RHSStripped) ||
- isa<ObjCEncodeExpr>(RHSStripped)) &&
- !LHSStripped->isNullPointerConstant(Context,
- Expr::NPC_ValueDependentIsNull)) {
- literalString = RHS.get();
- literalStringStripped = RHSStripped;
- }
-
- if (literalString) {
- DiagRuntimeBehavior(Loc, nullptr,
- PDiag(diag::warn_stringcompare)
- << isa<ObjCEncodeExpr>(literalStringStripped)
- << literalString->getSourceRange());
- }
- }
+ diagnoseTautologicalComparison(*this, Loc, LHS.get(), RHS.get(), Opc);
// C99 6.5.8p3 / C99 6.5.9p4
UsualArithmeticConversions(LHS, RHS);
@@ -10101,7 +10124,7 @@ QualType Sema::GetSignedVectorType(QualT
/// types.
QualType Sema::CheckVectorCompareOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc,
- bool IsRelational) {
+ BinaryOperatorKind Opc) {
// Check to make sure we're operating on vectors of the same type and width,
// Allowing one side to be a scalar of element type.
QualType vType = CheckVectorOperands(LHS, RHS, Loc, /*isCompAssign*/false,
@@ -10121,22 +10144,12 @@ QualType Sema::CheckVectorCompareOperand
// For non-floating point types, check for self-comparisons of the form
// x == x, x != x, x < x, etc. These always evaluate to a constant, and
// often indicate logic errors in the program.
- if (!LHSType->hasFloatingRepresentation() && !inTemplateInstantiation()) {
- if (DeclRefExpr* DRL
- = dyn_cast<DeclRefExpr>(LHS.get()->IgnoreParenImpCasts()))
- if (DeclRefExpr* DRR
- = dyn_cast<DeclRefExpr>(RHS.get()->IgnoreParenImpCasts()))
- if (DRL->getDecl() == DRR->getDecl())
- DiagRuntimeBehavior(Loc, nullptr,
- PDiag(diag::warn_comparison_always)
- << 0 // self-
- << 2 // "a constant"
- );
- }
+ diagnoseTautologicalComparison(*this, Loc, LHS.get(), RHS.get(), Opc);
// Check for comparisons of floating point operands using != and ==.
- if (!IsRelational && LHSType->hasFloatingRepresentation()) {
- assert (RHS.get()->getType()->hasFloatingRepresentation());
+ if (BinaryOperator::isEqualityOp(Opc) &&
+ LHSType->hasFloatingRepresentation()) {
+ assert(RHS.get()->getType()->hasFloatingRepresentation());
CheckFloatComparison(Loc, LHS.get(), RHS.get());
}
Modified: cfe/trunk/test/Sema/ext_vector_comparisons.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ext_vector_comparisons.c?rev=321972&r1=321971&r2=321972&view=diff
==============================================================================
--- cfe/trunk/test/Sema/ext_vector_comparisons.c (original)
+++ cfe/trunk/test/Sema/ext_vector_comparisons.c Sun Jan 7 13:57:48 2018
@@ -6,12 +6,12 @@ static int4 test1() {
int4 vec, rv;
// comparisons to self...
- return vec == vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec != vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec < vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec <= vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec > vec; // expected-warning{{self-comparison always evaluates to a constant}}
- return vec >= vec; // expected-warning{{self-comparison always evaluates to a constant}}
+ return vec == vec; // expected-warning{{self-comparison always evaluates to true}}
+ return vec != vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec < vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec <= vec; // expected-warning{{self-comparison always evaluates to true}}
+ return vec > vec; // expected-warning{{self-comparison always evaluates to false}}
+ return vec >= vec; // expected-warning{{self-comparison always evaluates to true}}
}
More information about the cfe-commits
mailing list