maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Factor out common tautological comparison code from scalar and vector compare checking. 

In passing, improve vector compare diagnostic to match scalar compare diagnostic.

llvm-svn: 321972
This commit is contained in:
Richard Smith 2018-01-07 21:57:48 +00:00
parent e9f44e1b80
commit abbb8ada45
4 changed files with 119 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@ -7906,7 +7906,7 @@ def note_ref_subobject_of_member_declared_here : Note<
// 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">,

3
clang/include/clang/Sema/Sema.h
View File

@ -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);

207
clang/lib/Sema/SemaExpr.cpp
View File

@ -9587,19 +9587,119 @@ static void diagnoseLogicalNotOnLHSofCheck(Sema &S, ExprResult &LHS,
// 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(ExprResult &LHS, ExprResult &RHS,
// 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(QualType V) {
/// 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::CheckVectorCompareOperands(ExprResult &LHS, ExprResult &RHS,
// 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());
}

12
clang/test/Sema/ext_vector_comparisons.c
View File

@ -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}}
}