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

Add a warning to diagnose statements in C++ like "*(volatile int*)x;". Conceptually, this is part of -Wunused-value, but I added a separate flag -Wunused-volatile-lvalue so it doesn't get turned off by accident with -Wno-unused-value. I also made a few minor improvements to existing unused value warnings in the process. <rdar://problem/11516811>. 

llvm-svn: 157362
This commit is contained in:
Eli Friedman 2012-05-24 00:47:05 +00:00
parent a95ce623d8
commit c11535c248
9 changed files with 124 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
clang/include/clang/AST/Expr.h
View File

@ -179,11 +179,12 @@ public:
SourceLocation getExprLoc() const LLVM_READONLY;
/// isUnusedResultAWarning - Return true if this immediate expression should
/// be warned about if the result is unused. If so, fill in Loc and Ranges
/// with location to warn on and the source range[s] to report with the
/// warning.
bool isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
SourceRange &R2, ASTContext &Ctx) const;
/// be warned about if the result is unused. If so, fill in expr, location,
/// and ranges with expr to warn on and source locations/ranges appropriate
/// for a warning.
bool isUnusedResultAWarning(const Expr *&WarnExpr, SourceLocation &Loc,
SourceRange &R1, SourceRange &R2,
ASTContext &Ctx) const;
/// isLValue - True if this expression is an "l-value" according to
/// the rules of the current language. C and C++ give somewhat

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

@ -4728,6 +4728,10 @@ def warn_unused_call : Warning<
def warn_unused_result : Warning<
"ignoring return value of function declared with warn_unused_result "
"attribute">, InGroup<DiagGroup<"unused-result">>;
def warn_unused_volatile : Warning<
"expression result unused; assign into a variable to force a volatile load">,
InGroup<DiagGroup<"unused-volatile-lvalue">>;
def warn_unused_comparison : Warning<
"%select{equality|inequality}0 comparison result unused">,
InGroup<UnusedComparison>;

121
clang/lib/AST/Expr.cpp
View File

@ -1654,8 +1654,9 @@ Stmt *BlockExpr::getBody() {
/// be warned about if the result is unused. If so, fill in Loc and Ranges
/// with location to warn on and the source range[s] to report with the
/// warning.
bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
SourceRange &R2, ASTContext &Ctx) const {
bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
SourceRange &R1, SourceRange &R2,
ASTContext &Ctx) const {
// Don't warn if the expr is type dependent. The type could end up
// instantiating to void.
if (isTypeDependent())
@ -1665,30 +1666,32 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
default:
if (getType()->isVoidType())
return false;
WarnE = this;
Loc = getExprLoc();
R1 = getSourceRange();
return true;
case ParenExprClass:
return cast<ParenExpr>(this)->getSubExpr()->
isUnusedResultAWarning(Loc, R1, R2, Ctx);
isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
case GenericSelectionExprClass:
return cast<GenericSelectionExpr>(this)->getResultExpr()->
isUnusedResultAWarning(Loc, R1, R2, Ctx);
isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
case UnaryOperatorClass: {
const UnaryOperator *UO = cast<UnaryOperator>(this);
switch (UO->getOpcode()) {
default: break;
case UO_Plus:
case UO_Minus:
case UO_AddrOf:
case UO_Not:
case UO_LNot:
case UO_Deref:
break;
case UO_PostInc:
case UO_PostDec:
case UO_PreInc:
case UO_PreDec: // ++/--
return false; // Not a warning.
case UO_Deref:
// Dereferencing a volatile pointer is a side-effect.
if (Ctx.getCanonicalType(getType()).isVolatileQualified())
return false;
break;
case UO_Real:
case UO_Imag:
// accessing a piece of a volatile complex is a side-effect.
@ -1697,8 +1700,9 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
return false;
break;
case UO_Extension:
return UO->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
return UO->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
}
WarnE = this;
Loc = UO->getOperatorLoc();
R1 = UO->getSubExpr()->getSourceRange();
return true;
@ -1717,17 +1721,18 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
dyn_cast<IntegerLiteral>(BO->getRHS()->IgnoreParens()))
if (IE->getValue() == 0)
return false;
return BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
return BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
// Consider '||', '&&' to have side effects if the LHS or RHS does.
case BO_LAnd:
case BO_LOr:
if (!BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx) ||
!BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx))
if (!BO->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) ||
!BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
return false;
break;
}
if (BO->isAssignmentOp())
return false;
WarnE = this;
Loc = BO->getOperatorLoc();
R1 = BO->getLHS()->getSourceRange();
R2 = BO->getRHS()->getSourceRange();
@ -1743,28 +1748,22 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
// be being used for control flow. Only warn if both the LHS and
// RHS are warnings.
const ConditionalOperator *Exp = cast<ConditionalOperator>(this);
if (!Exp->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx))
if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
return false;
if (!Exp->getLHS())
return true;
return Exp->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
}
case MemberExprClass:
// If the base pointer or element is to a volatile pointer/field, accessing
// it is a side effect.
if (Ctx.getCanonicalType(getType()).isVolatileQualified())
return false;
WarnE = this;
Loc = cast<MemberExpr>(this)->getMemberLoc();
R1 = SourceRange(Loc, Loc);
R2 = cast<MemberExpr>(this)->getBase()->getSourceRange();
return true;
case ArraySubscriptExprClass:
// If the base pointer or element is to a volatile pointer/field, accessing
// it is a side effect.
if (Ctx.getCanonicalType(getType()).isVolatileQualified())
return false;
WarnE = this;
Loc = cast<ArraySubscriptExpr>(this)->getRBracketLoc();
R1 = cast<ArraySubscriptExpr>(this)->getLHS()->getSourceRange();
R2 = cast<ArraySubscriptExpr>(this)->getRHS()->getSourceRange();
@ -1780,6 +1779,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
const CXXOperatorCallExpr *Op = cast<CXXOperatorCallExpr>(this);
if (Op->getOperator() == OO_EqualEqual ||
Op->getOperator() == OO_ExclaimEqual) {
WarnE = this;
Loc = Op->getOperatorLoc();
R1 = Op->getSourceRange();
return true;
@ -1800,6 +1800,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
// updated to match for QoI.
if (FD->getAttr<WarnUnusedResultAttr>() ||
FD->getAttr<PureAttr>() || FD->getAttr<ConstAttr>()) {
WarnE = this;
Loc = CE->getCallee()->getLocStart();
R1 = CE->getCallee()->getSourceRange();
@ -1824,6 +1825,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
ME->getSelector().getIdentifierInfoForSlot(0) &&
ME->getSelector().getIdentifierInfoForSlot(0)
->getName().startswith("init")) {
WarnE = this;
Loc = getExprLoc();
R1 = ME->getSourceRange();
return true;
@ -1831,6 +1833,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
const ObjCMethodDecl *MD = ME->getMethodDecl();
if (MD && MD->getAttr<WarnUnusedResultAttr>()) {
WarnE = this;
Loc = getExprLoc();
return true;
}
@ -1838,6 +1841,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
}
case ObjCPropertyRefExprClass:
WarnE = this;
Loc = getExprLoc();
R1 = getSourceRange();
return true;
@ -1850,6 +1854,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
isa<BinaryOperator>(PO->getSyntacticForm()))
return false;
WarnE = this;
Loc = getExprLoc();
R1 = getSourceRange();
return true;
@ -1864,50 +1869,70 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
const CompoundStmt *CS = cast<StmtExpr>(this)->getSubStmt();
if (!CS->body_empty()) {
if (const Expr *E = dyn_cast<Expr>(CS->body_back()))
return E->isUnusedResultAWarning(Loc, R1, R2, Ctx);
return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
if (const LabelStmt *Label = dyn_cast<LabelStmt>(CS->body_back()))
if (const Expr *E = dyn_cast<Expr>(Label->getSubStmt()))
return E->isUnusedResultAWarning(Loc, R1, R2, Ctx);
return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
}
if (getType()->isVoidType())
return false;
WarnE = this;
Loc = cast<StmtExpr>(this)->getLParenLoc();
R1 = getSourceRange();
return true;
}
case CStyleCastExprClass:
// If this is an explicit cast to void, allow it. People do this when they
// think they know what they're doing :).
if (getType()->isVoidType())
case CStyleCastExprClass: {
// Ignore an explicit cast to void, as long as the operand isn't a
// volatile lvalue.
const CStyleCastExpr *CE = cast<CStyleCastExpr>(this);
if (CE->getCastKind() == CK_ToVoid) {
if (CE->getSubExpr()->isGLValue() &&
CE->getSubExpr()->getType().isVolatileQualified())
return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
R1, R2, Ctx);
return false;
Loc = cast<CStyleCastExpr>(this)->getLParenLoc();
R1 = cast<CStyleCastExpr>(this)->getSubExpr()->getSourceRange();
}
WarnE = this;
Loc = CE->getLParenLoc();
R1 = CE->getSubExpr()->getSourceRange();
return true;
}
case CXXFunctionalCastExprClass: {
if (getType()->isVoidType())
// Ignore an explicit cast to void, as long as the operand isn't a
// volatile lvalue.
const CXXFunctionalCastExpr *CE = cast<CXXFunctionalCastExpr>(this);
if (CE->getCastKind() == CK_ToVoid) {
if (CE->getSubExpr()->isGLValue() &&
CE->getSubExpr()->getType().isVolatileQualified())
return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
R1, R2, Ctx);
return false;
const CastExpr *CE = cast<CastExpr>(this);
}
// If this is a cast to void or a constructor conversion, check the operand.
// If this is a cast to a constructor conversion, check the operand.
// Otherwise, the result of the cast is unused.
if (CE->getCastKind() == CK_ToVoid ||
CE->getCastKind() == CK_ConstructorConversion)
return (cast<CastExpr>(this)->getSubExpr()
->isUnusedResultAWarning(Loc, R1, R2, Ctx));
Loc = cast<CXXFunctionalCastExpr>(this)->getTypeBeginLoc();
R1 = cast<CXXFunctionalCastExpr>(this)->getSubExpr()->getSourceRange();
if (CE->getCastKind() == CK_ConstructorConversion)
return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
WarnE = this;
Loc = CE->getTypeBeginLoc();
R1 = CE->getSubExpr()->getSourceRange();
return true;
}
case ImplicitCastExprClass:
// Check the operand, since implicit casts are inserted by Sema
return (cast<ImplicitCastExpr>(this)
->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
case ImplicitCastExprClass: {
const CastExpr *ICE = cast<ImplicitCastExpr>(this);
// lvalue-to-rvalue conversion on a volatile lvalue is a side-effect.
if (ICE->getCastKind() == CK_LValueToRValue &&
ICE->getSubExpr()->getType().isVolatileQualified())
return false;
return ICE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
}
case CXXDefaultArgExprClass:
return (cast<CXXDefaultArgExpr>(this)
->getExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
->getExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx));
case CXXNewExprClass:
// FIXME: In theory, there might be new expressions that don't have side
@ -1916,10 +1941,10 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
return false;
case CXXBindTemporaryExprClass:
return (cast<CXXBindTemporaryExpr>(this)
->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx));
case ExprWithCleanupsClass:
return (cast<ExprWithCleanups>(this)
->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx));
}
}

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

@ -7412,8 +7412,6 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
// C99 6.5.17
static QualType CheckCommaOperands(Sema &S, ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc) {
S.DiagnoseUnusedExprResult(LHS.get());
LHS = S.CheckPlaceholderExpr(LHS.take());
RHS = S.CheckPlaceholderExpr(RHS.take());
if (LHS.isInvalid() || RHS.isInvalid())
@ -7429,6 +7427,8 @@ static QualType CheckCommaOperands(Sema &S, ExprResult &LHS, ExprResult &RHS,
if (LHS.isInvalid())
return QualType();
S.DiagnoseUnusedExprResult(LHS.get());
if (!S.getLangOpts().CPlusPlus) {
RHS = S.DefaultFunctionArrayLvalueConversion(RHS.take());
if (RHS.isInvalid())

10
clang/lib/Sema/SemaStmt.cpp
View File

@ -153,10 +153,11 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
if (!E)
return;
const Expr *WarnExpr;
SourceLocation Loc;
SourceRange R1, R2;
if (SourceMgr.isInSystemMacro(E->getExprLoc()) ||
!E->isUnusedResultAWarning(Loc, R1, R2, Context))
!E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, Context))
return;
// Okay, we have an unused result. Depending on what the base expression is,
@ -171,7 +172,7 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
if (DiagnoseUnusedComparison(*this, E))
return;
E = E->IgnoreParenImpCasts();
E = WarnExpr;
if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
if (E->getType()->isVoidType())
return;
@ -229,6 +230,11 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
}
}
if (E->isGLValue() && E->getType().isVolatileQualified()) {
Diag(Loc, diag::warn_unused_volatile) << R1 << R2;
return;
}
DiagRuntimeBehavior(Loc, 0, PDiag(DiagID) << R1 << R2);
}

1
clang/test/Sema/unused-expr.c
View File

@ -92,6 +92,7 @@ int t6() {
fn2(92, 21); // expected-warning {{ignoring return value of function declared with pure attribute}}
fn3(42); // expected-warning {{ignoring return value of function declared with const attribute}}
__builtin_fabsf(0); // expected-warning {{ignoring return value of function declared with const attribute}}
(void)0, fn1(); // expected-warning {{ignoring return value of function declared with warn_unused_result attribute}}
return 0;
}

2
clang/test/SemaCXX/reinterpret-cast.cpp
View File

@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify -ffreestanding -Wundefined-reinterpret-cast %s
// RUN: %clang_cc1 -fsyntax-only -verify -ffreestanding -Wundefined-reinterpret-cast -Wno-unused-volatile-lvalue %s
#include <stdint.h>

10
clang/test/SemaCXX/unused.cpp
View File

@ -1,5 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
// PR4103 : Make sure we don't get a bogus unused expression warning
namespace PR4103 {
class APInt {
char foo;
};
@ -22,3 +24,11 @@ struct X {
void test() {
X<int>();
}
}
namespace derefvolatile {
void f(volatile char* x) {
*x; // expected-warning {{expression result unused; assign into a variable to force a volatile load}}
(void)*x; // expected-warning {{expression result unused; assign into a variable to force a volatile load}}
}
}

2
clang/test/SemaCXX/warn-unused-value.cpp
View File

@ -12,7 +12,7 @@ namespace test0 {
// pointer to volatile has side effect (thus no warning)
Box* box = new Box;
box->i; // expected-warning {{expression result unused}}
box->j;
box->j; // expected-warning {{expression result unused}}
}
}