[cfe-commits] r157362 - in /cfe/trunk: include/clang/AST/Expr.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Expr.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaStmt.cpp test/Sema/unused-expr.c test/SemaCXX/reinterpret-cast.cpp test/SemaCXX/unused.cpp test/SemaCXX/warn-unused-value.cpp
Eli Friedman
eli.friedman at gmail.com
Wed May 23 17:47:06 PDT 2012
Author: efriedma
Date: Wed May 23 19:47:05 2012
New Revision: 157362
URL: http://llvm.org/viewvc/llvm-project?rev=157362&view=rev
Log:
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>.
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/Sema/unused-expr.c
cfe/trunk/test/SemaCXX/reinterpret-cast.cpp
cfe/trunk/test/SemaCXX/unused.cpp
cfe/trunk/test/SemaCXX/warn-unused-value.cpp
Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Wed May 23 19:47:05 2012
@@ -179,11 +179,12 @@
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
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 23 19:47:05 2012
@@ -4728,6 +4728,10 @@
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>;
Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Wed May 23 19:47:05 2012
@@ -1654,8 +1654,9 @@
/// 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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
ME->getSelector().getIdentifierInfoForSlot(0) &&
ME->getSelector().getIdentifierInfoForSlot(0)
->getName().startswith("init")) {
+ WarnE = this;
Loc = getExprLoc();
R1 = ME->getSourceRange();
return true;
@@ -1831,6 +1833,7 @@
const ObjCMethodDecl *MD = ME->getMethodDecl();
if (MD && MD->getAttr<WarnUnusedResultAttr>()) {
+ WarnE = this;
Loc = getExprLoc();
return true;
}
@@ -1838,6 +1841,7 @@
}
case ObjCPropertyRefExprClass:
+ WarnE = this;
Loc = getExprLoc();
R1 = getSourceRange();
return true;
@@ -1850,6 +1854,7 @@
isa<BinaryOperator>(PO->getSyntacticForm()))
return false;
+ WarnE = this;
Loc = getExprLoc();
R1 = getSourceRange();
return true;
@@ -1864,50 +1869,70 @@
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 @@
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));
}
}
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed May 23 19:47:05 2012
@@ -7412,8 +7412,6 @@
// 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 @@
if (LHS.isInvalid())
return QualType();
+ S.DiagnoseUnusedExprResult(LHS.get());
+
if (!S.getLangOpts().CPlusPlus) {
RHS = S.DefaultFunctionArrayLvalueConversion(RHS.take());
if (RHS.isInvalid())
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed May 23 19:47:05 2012
@@ -153,10 +153,11 @@
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 @@
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 @@
}
}
+ if (E->isGLValue() && E->getType().isVolatileQualified()) {
+ Diag(Loc, diag::warn_unused_volatile) << R1 << R2;
+ return;
+ }
+
DiagRuntimeBehavior(Loc, 0, PDiag(DiagID) << R1 << R2);
}
Modified: cfe/trunk/test/Sema/unused-expr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/unused-expr.c?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/test/Sema/unused-expr.c (original)
+++ cfe/trunk/test/Sema/unused-expr.c Wed May 23 19:47:05 2012
@@ -92,6 +92,7 @@
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;
}
Modified: cfe/trunk/test/SemaCXX/reinterpret-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/reinterpret-cast.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/reinterpret-cast.cpp (original)
+++ cfe/trunk/test/SemaCXX/reinterpret-cast.cpp Wed May 23 19:47:05 2012
@@ -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>
Modified: cfe/trunk/test/SemaCXX/unused.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/unused.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/unused.cpp (original)
+++ cfe/trunk/test/SemaCXX/unused.cpp Wed May 23 19:47:05 2012
@@ -1,24 +1,34 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
+
// PR4103 : Make sure we don't get a bogus unused expression warning
-class APInt {
- char foo;
-};
-class APSInt : public APInt {
- char bar;
-public:
- APSInt &operator=(const APSInt &RHS);
-};
+namespace PR4103 {
+ class APInt {
+ char foo;
+ };
+ class APSInt : public APInt {
+ char bar;
+ public:
+ APSInt &operator=(const APSInt &RHS);
+ };
-APSInt& APSInt::operator=(const APSInt &RHS) {
- APInt::operator=(RHS);
- return *this;
-}
+ APSInt& APSInt::operator=(const APSInt &RHS) {
+ APInt::operator=(RHS);
+ return *this;
+ }
-template<typename T>
-struct X {
- X();
-};
+ template<typename T>
+ struct X {
+ X();
+ };
+
+ void test() {
+ X<int>();
+ }
+}
-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}}
+ }
}
Modified: cfe/trunk/test/SemaCXX/warn-unused-value.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-value.cpp?rev=157362&r1=157361&r2=157362&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unused-value.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-value.cpp Wed May 23 19:47:05 2012
@@ -12,7 +12,7 @@
// 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}}
}
}
More information about the cfe-commits
mailing list