[clang] 00e5d38 - Do not warn that an expression of the form (void)arr; is unused when
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 17:40:47 PDT 2020
Author: Richard Smith
Date: 2020-05-27T17:26:29-07:00
New Revision: 00e5d38d40162d049f67b436ad42c9d05092e65c
URL: https://github.com/llvm/llvm-project/commit/00e5d38d40162d049f67b436ad42c9d05092e65c
DIFF: https://github.com/llvm/llvm-project/commit/00e5d38d40162d049f67b436ad42c9d05092e65c.diff
LOG: Do not warn that an expression of the form (void)arr; is unused when
arr is a volatile non-local array.
This fixes a recent regression exposed by removing lvalue-to-rvalue
conversion of discarded volatile arrays. In passing, regularize the
rules we use to determine whether '(void)expr;' warns when expr is a
volatile glvalue.
Added:
Modified:
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/warn-unused-value.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 0ca4941789e7..deca0b82c4e3 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -232,6 +232,11 @@ class Expr : public ValueStmt {
/// a problem with a generic expression.
SourceLocation getExprLoc() const LLVM_READONLY;
+ /// Determine whether an lvalue-to-rvalue conversion should implicitly be
+ /// applied to this expression if it appears as a discarded-value expression
+ /// in C++11 onwards. This applies to certain forms of volatile glvalues.
+ bool isReadIfDiscardedInCPlusPlus11() const;
+
/// isUnusedResultAWarning - Return true if this immediate expression should
/// 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
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 4c175fff6421..feb0517204c4 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2267,6 +2267,64 @@ Stmt *BlockExpr::getBody() {
// Generic Expression Routines
//===----------------------------------------------------------------------===//
+bool Expr::isReadIfDiscardedInCPlusPlus11() const {
+ // In C++11, discarded-value expressions of a certain form are special,
+ // according to [expr]p10:
+ // The lvalue-to-rvalue conversion (4.1) is applied only if the
+ // expression is an lvalue of volatile-qualified type and it has
+ // one of the following forms:
+ if (!isGLValue() || !getType().isVolatileQualified())
+ return false;
+
+ const Expr *E = IgnoreParens();
+
+ // - id-expression (5.1.1),
+ if (isa<DeclRefExpr>(E))
+ return true;
+
+ // - subscripting (5.2.1),
+ if (isa<ArraySubscriptExpr>(E))
+ return true;
+
+ // - class member access (5.2.5),
+ if (isa<MemberExpr>(E))
+ return true;
+
+ // - indirection (5.3.1),
+ if (auto *UO = dyn_cast<UnaryOperator>(E))
+ if (UO->getOpcode() == UO_Deref)
+ return true;
+
+ if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+ // - pointer-to-member operation (5.5),
+ if (BO->isPtrMemOp())
+ return true;
+
+ // - comma expression (5.18) where the right operand is one of the above.
+ if (BO->getOpcode() == BO_Comma)
+ return BO->getRHS()->isReadIfDiscardedInCPlusPlus11();
+ }
+
+ // - conditional expression (5.16) where both the second and the third
+ // operands are one of the above, or
+ if (auto *CO = dyn_cast<ConditionalOperator>(E))
+ return CO->getTrueExpr()->isReadIfDiscardedInCPlusPlus11() &&
+ CO->getFalseExpr()->isReadIfDiscardedInCPlusPlus11();
+ // The related edge case of "*x ?: *x".
+ if (auto *BCO =
+ dyn_cast<BinaryConditionalOperator>(E)) {
+ if (auto *OVE = dyn_cast<OpaqueValueExpr>(BCO->getTrueExpr()))
+ return OVE->getSourceExpr()->isReadIfDiscardedInCPlusPlus11() &&
+ BCO->getFalseExpr()->isReadIfDiscardedInCPlusPlus11();
+ }
+
+ // Objective-C++ extensions to the rule.
+ if (isa<PseudoObjectExpr>(E) || isa<ObjCIvarRefExpr>(E))
+ return true;
+
+ return false;
+}
+
/// 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
@@ -2555,20 +2613,31 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
}
case CXXFunctionalCastExprClass:
case CStyleCastExprClass: {
- // Ignore an explicit cast to void unless the operand is a non-trivial
- // volatile lvalue.
+ // Ignore an explicit cast to void, except in C++98 if the operand is a
+ // volatile glvalue for which we would trigger an implicit read in any
+ // other language mode. (Such an implicit read always happens as part of
+ // the lvalue conversion in C, and happens in C++ for expressions of all
+ // forms where it seems likely the user intended to trigger a volatile
+ // load.)
const CastExpr *CE = cast<CastExpr>(this);
+ const Expr *SubE = CE->getSubExpr()->IgnoreParens();
if (CE->getCastKind() == CK_ToVoid) {
- if (CE->getSubExpr()->isGLValue() &&
- CE->getSubExpr()->getType().isVolatileQualified()) {
- const DeclRefExpr *DRE =
- dyn_cast<DeclRefExpr>(CE->getSubExpr()->IgnoreParens());
- if (!(DRE && isa<VarDecl>(DRE->getDecl()) &&
- cast<VarDecl>(DRE->getDecl())->hasLocalStorage()) &&
- !isa<CallExpr>(CE->getSubExpr()->IgnoreParens())) {
- return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
- R1, R2, Ctx);
- }
+ if (Ctx.getLangOpts().CPlusPlus && !Ctx.getLangOpts().CPlusPlus11 &&
+ SubE->isReadIfDiscardedInCPlusPlus11()) {
+ // Suppress the "unused value" warning for idiomatic usage of
+ // '(void)var;' used to suppress "unused variable" warnings.
+ if (auto *DRE = dyn_cast<DeclRefExpr>(SubE))
+ if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+ if (!VD->isExternallyVisible())
+ return false;
+
+ // The lvalue-to-rvalue conversion would have no effect for an array.
+ // It's implausible that the programmer expected this to result in a
+ // volatile array load, so don't warn.
+ if (SubE->getType()->isArrayType())
+ return false;
+
+ return SubE->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
}
return false;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 7cda60ba7598..b655c8281689 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7660,61 +7660,6 @@ ExprResult Sema::ActOnNoexceptExpr(SourceLocation KeyLoc, SourceLocation,
return BuildCXXNoexceptExpr(KeyLoc, Operand, RParen);
}
-static bool IsSpecialDiscardedValue(Expr *E) {
- // In C++11, discarded-value expressions of a certain form are special,
- // according to [expr]p10:
- // The lvalue-to-rvalue conversion (4.1) is applied only if the
- // expression is an lvalue of volatile-qualified type and it has
- // one of the following forms:
- E = E->IgnoreParens();
-
- // - id-expression (5.1.1),
- if (isa<DeclRefExpr>(E))
- return true;
-
- // - subscripting (5.2.1),
- if (isa<ArraySubscriptExpr>(E))
- return true;
-
- // - class member access (5.2.5),
- if (isa<MemberExpr>(E))
- return true;
-
- // - indirection (5.3.1),
- if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E))
- if (UO->getOpcode() == UO_Deref)
- return true;
-
- if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
- // - pointer-to-member operation (5.5),
- if (BO->isPtrMemOp())
- return true;
-
- // - comma expression (5.18) where the right operand is one of the above.
- if (BO->getOpcode() == BO_Comma)
- return IsSpecialDiscardedValue(BO->getRHS());
- }
-
- // - conditional expression (5.16) where both the second and the third
- // operands are one of the above, or
- if (ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E))
- return IsSpecialDiscardedValue(CO->getTrueExpr()) &&
- IsSpecialDiscardedValue(CO->getFalseExpr());
- // The related edge case of "*x ?: *x".
- if (BinaryConditionalOperator *BCO =
- dyn_cast<BinaryConditionalOperator>(E)) {
- if (OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(BCO->getTrueExpr()))
- return IsSpecialDiscardedValue(OVE->getSourceExpr()) &&
- IsSpecialDiscardedValue(BCO->getFalseExpr());
- }
-
- // Objective-C++ extensions to the rule.
- if (isa<PseudoObjectExpr>(E) || isa<ObjCIvarRefExpr>(E))
- return true;
-
- return false;
-}
-
/// Perform the conversions required for an expression used in a
/// context that ignores the result.
ExprResult Sema::IgnoredValueConversions(Expr *E) {
@@ -7739,23 +7684,20 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
return E;
}
- if (getLangOpts().CPlusPlus) {
+ if (getLangOpts().CPlusPlus) {
// The C++11 standard defines the notion of a discarded-value expression;
// normally, we don't need to do anything to handle it, but if it is a
// volatile lvalue with a special form, we perform an lvalue-to-rvalue
// conversion.
- if (getLangOpts().CPlusPlus11 && E->isGLValue() &&
- E->getType().isVolatileQualified()) {
- if (IsSpecialDiscardedValue(E)) {
- ExprResult Res = DefaultLvalueConversion(E);
- if (Res.isInvalid())
- return E;
- E = Res.get();
- } else {
- // Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if
- // it occurs as a discarded-value expression.
- CheckUnusedVolatileAssignment(E);
- }
+ if (getLangOpts().CPlusPlus11 && E->isReadIfDiscardedInCPlusPlus11()) {
+ ExprResult Res = DefaultLvalueConversion(E);
+ if (Res.isInvalid())
+ return E;
+ E = Res.get();
+ } else {
+ // Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if
+ // it occurs as a discarded-value expression.
+ CheckUnusedVolatileAssignment(E);
}
// C++1z:
diff --git a/clang/test/SemaCXX/warn-unused-value.cpp b/clang/test/SemaCXX/warn-unused-value.cpp
index 98e2a4e86304..02bceeca1337 100644
--- a/clang/test/SemaCXX/warn-unused-value.cpp
+++ b/clang/test/SemaCXX/warn-unused-value.cpp
@@ -108,3 +108,33 @@ void f() {
(void)sizeof(*x); // Ok
}
}
+
+static volatile char var1 = 'a';
+volatile char var2 = 'a';
+static volatile char arr1[] = "hello";
+volatile char arr2[] = "hello";
+void volatile_array() {
+ static volatile char var3 = 'a';
+ volatile char var4 = 'a';
+ static volatile char arr3[] = "hello";
+ volatile char arr4[] = "hello";
+
+ // These all result in volatile loads in C and C++11. In C++98, they don't,
+ // but we suppress the warning in the case where '(void)var;' might be
+ // idiomatically suppressing an 'unused variable' warning.
+ (void)var1;
+ (void)var2;
+#if __cplusplus < 201103L
+ // expected-warning at -2 {{expression result unused; assign into a variable to force a volatile load}}
+#endif
+ (void)var3;
+ (void)var4;
+
+ // None of these result in volatile loads in any language mode, and it's not
+ // really reasonable to assume that they would, since volatile array loads
+ // don't really exist anywhere.
+ (void)arr1;
+ (void)arr2;
+ (void)arr3;
+ (void)arr4;
+}
More information about the cfe-commits
mailing list