Author: rsmith Date: Tue Oct 8 19:04:54 2019 New Revision: 374135 URL: http://llvm.org/viewvc/llvm-project?rev=374135&view=rev Log: [c++20] P1152R4: warn on any simple-assignment to a volatile lvalue whose value is not ignored.
We don't warn on all the cases that are deprecated: specifically, we choose to not warn for now if there are parentheses around the assignment but its value is not actually used. This seems like a more defensible rule, particularly for cases like sizeof(v = a), where the parens are part of the operand rather than the sizeof syntax. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/test/SemaCXX/deprecated.cpp cfe/trunk/www/cxx_status.html Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 8 19:04:54 2019 @@ -6654,6 +6654,9 @@ def err_increment_decrement_enum : Error def warn_deprecated_increment_decrement_volatile : Warning< "%select{decrement|increment}0 of object of volatile-qualified type %1 " "is deprecated">, InGroup<DeprecatedVolatile>; +def warn_deprecated_simple_assign_volatile : Warning< + "use of result of assignment to object of volatile-qualified type %0 " + "is deprecated">, InGroup<DeprecatedVolatile>; def warn_deprecated_compound_assign_volatile : Warning< "compound assignment to object of volatile-qualified type %0 is deprecated">, InGroup<DeprecatedVolatile>; Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct 8 19:04:54 2019 @@ -1062,6 +1062,11 @@ public: llvm::SmallPtrSet<const Expr *, 8> PossibleDerefs; + /// Expressions appearing as the LHS of a volatile assignment in this + /// context. We produce a warning for these when popping the context if + /// they are not discarded-value expressions nor unevaluated operands. + SmallVector<Expr*, 2> VolatileAssignmentLHSs; + /// \brief Describes whether we are in an expression constext which we have /// to handle differently. enum ExpressionKind { @@ -4248,6 +4253,9 @@ public: ExprResult TransformToPotentiallyEvaluated(Expr *E); ExprResult HandleExprEvaluationContextForTypeof(Expr *E); + ExprResult CheckUnevaluatedOperand(Expr *E); + void CheckUnusedVolatileAssignment(Expr *E); + ExprResult ActOnConstantExpression(ExprResult Res); // Functions for marking a declaration referenced. These functions also Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct 8 19:04:54 2019 @@ -3801,6 +3801,16 @@ bool Sema::CheckUnaryExprOrTypeTraitOper QualType ExprTy = E->getType(); assert(!ExprTy->isReferenceType()); + bool IsUnevaluatedOperand = + (ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf || + ExprKind == UETT_PreferredAlignOf); + if (IsUnevaluatedOperand) { + ExprResult Result = CheckUnevaluatedOperand(E); + if (Result.isInvalid()) + return true; + E = Result.get(); + } + if (ExprKind == UETT_VecStep) return CheckVecStepTraitOperandType(*this, ExprTy, E->getExprLoc(), E->getSourceRange()); @@ -3838,9 +3848,8 @@ bool Sema::CheckUnaryExprOrTypeTraitOper // The operand for sizeof and alignof is in an unevaluated expression context, // so side effects could result in unintended consequences. - if ((ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf || - ExprKind == UETT_PreferredAlignOf) && - !inTemplateInstantiation() && E->HasSideEffects(Context, false)) + if (IsUnevaluatedOperand && !inTemplateInstantiation() && + E->HasSideEffects(Context, false)) Diag(E->getExprLoc(), diag::warn_side_effects_unevaluated_context); if (CheckObjCTraitOperandConstraints(*this, ExprTy, E->getExprLoc(), @@ -3939,8 +3948,6 @@ bool Sema::CheckUnaryExprOrTypeTraitOper } static bool CheckAlignOfExpr(Sema &S, Expr *E, UnaryExprOrTypeTrait ExprKind) { - E = E->IgnoreParens(); - // Cannot know anything else if the expression is dependent. if (E->isTypeDependent()) return false; @@ -3952,9 +3959,10 @@ static bool CheckAlignOfExpr(Sema &S, Ex } ValueDecl *D = nullptr; - if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) { + Expr *Inner = E->IgnoreParens(); + if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Inner)) { D = DRE->getDecl(); - } else if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) { + } else if (MemberExpr *ME = dyn_cast<MemberExpr>(Inner)) { D = ME->getMemberDecl(); } @@ -11944,7 +11952,7 @@ QualType Sema::CheckAssignmentOperands(E // A simple-assignment whose left operand is of a volatile-qualified // type is deprecated unless the assignment is either a discarded-value // expression or an unevaluated operand - // FIXME: Implement checks for this. + ExprEvalContexts.back().VolatileAssignmentLHSs.push_back(LHSExpr); } else { // C++2a [expr.ass]p6: // [Compound-assignment] expressions are deprecated if E1 has @@ -15082,6 +15090,26 @@ void Sema::WarnOnPendingNoDerefs(Express Rec.PossibleDerefs.clear(); } +/// Check whether E, which is either a discarded-value expression or an +/// unevaluated operand, is a simple-assignment to a volatlie-qualified lvalue, +/// and if so, remove it from the list of volatile-qualified assignments that +/// we are going to warn are deprecated. +void Sema::CheckUnusedVolatileAssignment(Expr *E) { + if (!E->getType().isVolatileQualified() || !getLangOpts().CPlusPlus2a) + return; + + // Note: ignoring parens here is not justified by the standard rules, but + // ignoring parentheses seems like a more reasonable approach, and this only + // drives a deprecation warning so doesn't affect conformance. + if (auto *BO = dyn_cast<BinaryOperator>(E->IgnoreParenImpCasts())) { + if (BO->getOpcode() == BO_Assign) { + auto &LHSs = ExprEvalContexts.back().VolatileAssignmentLHSs; + LHSs.erase(std::remove(LHSs.begin(), LHSs.end(), BO->getLHS()), + LHSs.end()); + } + } +} + void Sema::PopExpressionEvaluationContext() { ExpressionEvaluationContextRecord& Rec = ExprEvalContexts.back(); unsigned NumTypos = Rec.NumTypos; @@ -15116,6 +15144,13 @@ void Sema::PopExpressionEvaluationContex WarnOnPendingNoDerefs(Rec); + // Warn on any volatile-qualified simple-assignments that are not discarded- + // value expressions nor unevaluated operands (those cases get removed from + // this list by CheckUnusedVolatileAssignment). + for (auto *BO : Rec.VolatileAssignmentLHSs) + Diag(BO->getBeginLoc(), diag::warn_deprecated_simple_assign_volatile) + << BO->getType(); + // When are coming out of an unevaluated context, clear out any // temporaries that we may have created as part of the evaluation of // the expression in that context: they aren't relevant because they Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Oct 8 19:04:54 2019 @@ -499,6 +499,11 @@ ExprResult Sema::BuildCXXTypeId(QualType } } + ExprResult Result = CheckUnevaluatedOperand(E); + if (Result.isInvalid()) + return ExprError(); + E = Result.get(); + // C++ [expr.typeid]p4: // [...] If the type of the type-id is a reference to a possibly // cv-qualified type, the result of the typeid expression refers to a @@ -6609,6 +6614,11 @@ ExprResult Sema::ActOnDecltypeExpression ExprEvalContexts.back().ExprContext = ExpressionEvaluationContextRecord::EK_Other; + Result = CheckUnevaluatedOperand(E); + if (Result.isInvalid()) + return ExprError(); + E = Result.get(); + // In MS mode, don't perform any extra checking of call return types within a // decltype expression. if (getLangOpts().MSVCCompat) @@ -7233,7 +7243,10 @@ ExprResult Sema::BuildCXXNoexceptExpr(So if (R.isInvalid()) return R; - // The operand may have been modified when checking the placeholder type. + R = CheckUnevaluatedOperand(R.get()); + if (R.isInvalid()) + return ExprError(); + Operand = R.get(); if (!inTemplateInstantiation() && Operand->HasSideEffects(Context, false)) { @@ -7337,12 +7350,17 @@ ExprResult Sema::IgnoredValueConversions // volatile lvalue with a special form, we perform an lvalue-to-rvalue // conversion. if (getLangOpts().CPlusPlus11 && E->isGLValue() && - E->getType().isVolatileQualified() && - IsSpecialDiscardedValue(E)) { - ExprResult Res = DefaultLvalueConversion(E); - if (Res.isInvalid()) - return E; - E = Res.get(); + 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); + } } // C++1z: @@ -7377,6 +7395,14 @@ ExprResult Sema::IgnoredValueConversions return E; } +ExprResult Sema::CheckUnevaluatedOperand(Expr *E) { + // Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if + // it occurs as an unevaluated operand. + CheckUnusedVolatileAssignment(E); + + return E; +} + // If we can unambiguously determine whether Var can never be used // in a constant expression, return true. // - if the variable and its initializer are non-dependent, then Modified: cfe/trunk/test/SemaCXX/deprecated.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/deprecated.cpp?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/deprecated.cpp (original) +++ cfe/trunk/test/SemaCXX/deprecated.cpp Tue Oct 8 19:04:54 2019 @@ -1,13 +1,17 @@ -// RUN: %clang_cc1 -std=c++98 %s -Wdeprecated -verify -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++14 %s -Wdeprecated -verify -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++17 %s -Wdeprecated -verify -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++2a %s -Wdeprecated -verify=expected,cxx20 -triple x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++98 %s -Wno-parentheses -Wdeprecated -verify -triple x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++11 %s -Wno-parentheses -Wdeprecated -verify -triple x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++14 %s -Wno-parentheses -Wdeprecated -verify -triple x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++17 %s -Wno-parentheses -Wdeprecated -verify -triple x86_64-linux-gnu +// RUN: %clang_cc1 -std=c++2a %s -Wno-parentheses -Wdeprecated -verify=expected,cxx20 -triple x86_64-linux-gnu -// RUN: %clang_cc1 -std=c++14 %s -Wdeprecated -verify -triple x86_64-linux-gnu -Wno-deprecated-register -DNO_DEPRECATED_FLAGS +// RUN: %clang_cc1 -std=c++14 %s -Wno-parentheses -Wdeprecated -verify -triple x86_64-linux-gnu -Wno-deprecated-register -DNO_DEPRECATED_FLAGS #include "Inputs/register.h" +namespace std { + struct type_info {}; +} + void g() throw(); void h() throw(int); void i() throw(...); @@ -133,17 +137,36 @@ namespace DeprecatedVolatile { n = 5; // ok #if __cplusplus >= 201103L decltype(n = 5) m = n; // ok expected-warning {{side effects}} - m = sizeof(n = 5); // ok expected-warning {{side effects}} + (void)noexcept(n = 5); // ok expected-warning {{side effects}} #endif + (void)typeid(n = 5); // ok expected-warning {{side effects}} (n = 5, 0); // ok - use(n = 5); // FIXME: deprecated - (n = 5); // FIXME: deprecated - int q = n = 5; // FIXME: deprecated - q = n = 5; // FIXME: deprecated + use(n = 5); // cxx20-warning {{use of result of assignment to object of volatile-qualified type 'volatile int' is deprecated}} + int q = n = 5; // cxx20-warning {{deprecated}} + q = n = 5; // cxx20-warning {{deprecated}} #if __cplusplus >= 201103L - decltype(q = n = 5) m2 = q; // FIXME: deprecated expected-warning {{side effects}} + decltype(q = n = 5) m2 = q; // cxx20-warning {{deprecated}} expected-warning {{side effects}} + (void)noexcept(q = n = 5); // cxx20-warning {{deprecated}} expected-warning {{side effects}} #endif - q = sizeof(q = n = 5); // FIXME: deprecated expected-warning {{side effects}} + (void)sizeof(q = n = 5); // cxx20-warning {{deprecated}} expected-warning {{side effects}} + (void)typeid(use(n = 5)); // cxx20-warning {{deprecated}} expected-warning {{side effects}} + (void)__alignof(+(n = 5)); // cxx20-warning {{deprecated}} expected-warning {{side effects}} + + // FIXME: These cases are technically deprecated because the parens are + // part of the operand, but we choose to not diagnose for now. + (void)sizeof(n = 5); // expected-warning {{side effects}} + (void)__alignof(n = 5); // expected-warning {{side effects}} + // Similarly here. + (n = 5); + + volatile bool b = true; + if (b = true) {} // cxx20-warning {{deprecated}} + for (b = true; + b = true; // cxx20-warning {{deprecated}} + b = true) {} + for (volatile bool x = true; + volatile bool y = true; // ok despite volatile load from volatile initialization + ) {} // inc / dec / compound assignments are always deprecated ++n; // cxx20-warning {{increment of object of volatile-qualified type 'volatile int' is deprecated}} Modified: cfe/trunk/www/cxx_status.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=374135&r1=374134&r2=374135&view=diff ============================================================================== --- cfe/trunk/www/cxx_status.html (original) +++ cfe/trunk/www/cxx_status.html Tue Oct 8 19:04:54 2019 @@ -1103,7 +1103,7 @@ as the draft C++2a standard evolves. <tr> <td>Deprecate some problematic uses of <tt>volatile</tt></td> <td><a href="http://wg21.link/p1152r4">P1152R4</a></td> - <td class="partial" align="center">Partial</td> + <td class="svn" align="center">SVN</td> </tr> <tr> <td><tt>[[nodiscard("with reason")]]</tt></td> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits