llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) <details> <summary>Changes</summary> Functions referenced in discarded statements could be treated as odr-used because we did not properly set the correct evaluation context in some places. Fixes https://github.com/llvm/llvm-project/issues/140449 --- Full diff: https://github.com/llvm/llvm-project/pull/140576.diff 8 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/include/clang/Sema/EnterExpressionEvaluationContext.h (+16) - (modified) clang/include/clang/Sema/Sema.h (+11-18) - (modified) clang/lib/Sema/SemaCoroutine.cpp (+4-2) - (modified) clang/lib/Sema/SemaDecl.cpp (+2-18) - (modified) clang/lib/Sema/SemaExpr.cpp (+58-27) - (modified) clang/lib/Sema/SemaLambda.cpp (+2-8) - (modified) clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp (+33) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0c12091a90add..dde21b7f9e15d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -738,6 +738,7 @@ Bug Fixes to C++ Support - Fixed a function declaration mismatch that caused inconsistencies between concepts and variable template declarations. (#GH139476) - Clang no longer segfaults when there is a configuration mismatch between modules and their users (http://crbug.com/400353616). - Fix an incorrect deduction when calling an explicit object member function template through an overload set address. +- Clang could incorrectly instantiate functions in discarded contexts (#GH140449) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/EnterExpressionEvaluationContext.h b/clang/include/clang/Sema/EnterExpressionEvaluationContext.h index 5eca797b8842b..7b5415f34a008 100644 --- a/clang/include/clang/Sema/EnterExpressionEvaluationContext.h +++ b/clang/include/clang/Sema/EnterExpressionEvaluationContext.h @@ -64,6 +64,22 @@ class EnterExpressionEvaluationContext { } }; +/// RAII object that enters a new function expression evaluation context. +class EnterExpressionEvaluationContextForFunction { + Sema &Actions; + +public: + EnterExpressionEvaluationContextForFunction( + Sema &Actions, Sema::ExpressionEvaluationContext NewContext, + FunctionDecl *FD = nullptr) + : Actions(Actions) { + Actions.PushExpressionEvaluationContextForFunction(NewContext, FD); + } + ~EnterExpressionEvaluationContextForFunction() { + Actions.PopExpressionEvaluationContext(); + } +}; + } // namespace clang #endif diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5ec67087aeea4..9af3387b533c0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6825,8 +6825,9 @@ class Sema final : public SemaBase { bool isDiscardedStatementContext() const { return Context == ExpressionEvaluationContext::DiscardedStatement || - (Context == - ExpressionEvaluationContext::ImmediateFunctionContext && + ((Context == + ExpressionEvaluationContext::ImmediateFunctionContext || + isPotentiallyEvaluated()) && InDiscardedStatement); } }; @@ -6915,6 +6916,10 @@ class Sema final : public SemaBase { ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl = nullptr, ExpressionEvaluationContextRecord::ExpressionKind Type = ExpressionEvaluationContextRecord::EK_Other); + + void PushExpressionEvaluationContextForFunction( + ExpressionEvaluationContext NewContext, FunctionDecl *FD); + enum ReuseLambdaContextDecl_t { ReuseLambdaContextDecl }; void PushExpressionEvaluationContext( ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t, @@ -13371,23 +13376,11 @@ class Sema final : public SemaBase { : S(S), SavedContext(S, DC) { auto *FD = dyn_cast<FunctionDecl>(DC); S.PushFunctionScope(); - S.PushExpressionEvaluationContext( - (FD && FD->isImmediateFunction()) - ? ExpressionEvaluationContext::ImmediateFunctionContext - : ExpressionEvaluationContext::PotentiallyEvaluated); - if (FD) { - auto &Current = S.currentEvaluationContext(); - const auto &Parent = S.parentEvaluationContext(); - + S.PushExpressionEvaluationContextForFunction( + ExpressionEvaluationContext::PotentiallyEvaluated, FD); + if (FD) FD->setWillHaveBody(true); - Current.InImmediateFunctionContext = - FD->isImmediateFunction() || - (isLambdaMethod(FD) && (Parent.isConstantEvaluated() || - Parent.isImmediateFunctionContext())); - - Current.InImmediateEscalatingFunctionContext = - S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating(); - } else + else assert(isa<ObjCMethodDecl>(DC)); } diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index 279e4c77d04aa..425b32e53a7b7 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -697,8 +697,10 @@ static void checkReturnStmtInCoroutine(Sema &S, FunctionScopeInfo *FSI) { bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc, StringRef Keyword) { // Ignore previous expr evaluation contexts. - EnterExpressionEvaluationContext PotentiallyEvaluated( - *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated); + EnterExpressionEvaluationContextForFunction PotentiallyEvaluated( + *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, + dyn_cast_or_null<FunctionDecl>(CurContext)); + if (!checkCoroutineContext(*this, KWLoc, Keyword)) return false; auto *ScopeInfo = getCurFunction(); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index a7d59ec232b64..02a0ed2172bd7 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15866,24 +15866,8 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D, // Do not push if it is a lambda because one is already pushed when building // the lambda in ActOnStartOfLambdaDefinition(). if (!isLambdaCallOperator(FD)) - // [expr.const]/p14.1 - // An expression or conversion is in an immediate function context if it is - // potentially evaluated and either: its innermost enclosing non-block scope - // is a function parameter scope of an immediate function. - PushExpressionEvaluationContext( - FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext - : ExprEvalContexts.back().Context); - - // Each ExpressionEvaluationContextRecord also keeps track of whether the - // context is nested in an immediate function context, so smaller contexts - // that appear inside immediate functions (like variable initializers) are - // considered to be inside an immediate function context even though by - // themselves they are not immediate function contexts. But when a new - // function is entered, we need to reset this tracking, since the entered - // function might be not an immediate function. - ExprEvalContexts.back().InImmediateFunctionContext = FD->isConsteval(); - ExprEvalContexts.back().InImmediateEscalatingFunctionContext = - getLangOpts().CPlusPlus20 && FD->isImmediateEscalating(); + PushExpressionEvaluationContextForFunction(ExprEvalContexts.back().Context, + FD); // Check for defining attributes before the check for redefinition. if (const auto *Attr = FD->getAttr<AliasAttr>()) { diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 91e63c7cb8677..cca9f1ea5981c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -17749,6 +17749,47 @@ Sema::PushExpressionEvaluationContext( PushExpressionEvaluationContext(NewContext, ClosureContextDecl, ExprContext); } +void Sema::PushExpressionEvaluationContextForFunction( + ExpressionEvaluationContext NewContext, FunctionDecl *FD) { + // [expr.const]/p14.1 + // An expression or conversion is in an immediate function context if it is + // potentially evaluated and either: its innermost enclosing non-block scope + // is a function parameter scope of an immediate function. + PushExpressionEvaluationContext( + FD && FD->isConsteval() + ? ExpressionEvaluationContext::ImmediateFunctionContext + : NewContext); + const Sema::ExpressionEvaluationContextRecord &Parent = + parentEvaluationContext(); + Sema::ExpressionEvaluationContextRecord &Current = currentEvaluationContext(); + + Current.InDiscardedStatement = false; + + if (FD) { + + // Each ExpressionEvaluationContextRecord also keeps track of whether the + // context is nested in an immediate function context, so smaller contexts + // that appear inside immediate functions (like variable initializers) are + // considered to be inside an immediate function context even though by + // themselves they are not immediate function contexts. But when a new + // function is entered, we need to reset this tracking, since the entered + // function might be not an immediate function. + + Current.InImmediateEscalatingFunctionContext = + getLangOpts().CPlusPlus20 && FD->isImmediateEscalating(); + + if (isLambdaMethod(FD)) { + Current.InDiscardedStatement = Parent.isDiscardedStatementContext(); + Current.InImmediateFunctionContext = + FD->isConsteval() || + (isLambdaMethod(FD) && (Parent.isConstantEvaluated() || + Parent.isImmediateFunctionContext())); + } else { + Current.InImmediateFunctionContext = FD->isConsteval(); + } + } +} + namespace { const DeclRefExpr *CheckPossibleDeref(Sema &S, const Expr *PossibleDeref) { @@ -18361,35 +18402,23 @@ enum class OdrUseContext { /// Are we within a context in which references to resolved functions or to /// variables result in odr-use? static OdrUseContext isOdrUseContext(Sema &SemaRef) { - OdrUseContext Result; - - switch (SemaRef.ExprEvalContexts.back().Context) { - case Sema::ExpressionEvaluationContext::Unevaluated: - case Sema::ExpressionEvaluationContext::UnevaluatedList: - case Sema::ExpressionEvaluationContext::UnevaluatedAbstract: - return OdrUseContext::None; - - case Sema::ExpressionEvaluationContext::ConstantEvaluated: - case Sema::ExpressionEvaluationContext::ImmediateFunctionContext: - case Sema::ExpressionEvaluationContext::PotentiallyEvaluated: - Result = OdrUseContext::Used; - break; + const Sema::ExpressionEvaluationContextRecord &Context = + SemaRef.currentEvaluationContext(); - case Sema::ExpressionEvaluationContext::DiscardedStatement: - Result = OdrUseContext::FormallyOdrUsed; - break; - - case Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed: - // A default argument formally results in odr-use, but doesn't actually - // result in a use in any real sense until it itself is used. - Result = OdrUseContext::FormallyOdrUsed; - break; - } + if (Context.isUnevaluated()) + return OdrUseContext::None; if (SemaRef.CurContext->isDependentContext()) return OdrUseContext::Dependent; - return Result; + if (Context.isDiscardedStatementContext()) + return OdrUseContext::FormallyOdrUsed; + + else if (Context.Context == + Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed) + return OdrUseContext::FormallyOdrUsed; + + return OdrUseContext::Used; } static bool isImplicitlyDefinableConstexprFunction(FunctionDecl *Func) { @@ -20356,9 +20385,11 @@ MarkExprReferenced(Sema &SemaRef, SourceLocation Loc, Decl *D, Expr *E, } void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const Expr *Base) { - // TODO: update this with DR# once a defect report is filed. - // C++11 defect. The address of a pure member should not be an ODR use, even - // if it's a qualified reference. + // [basic.def.odr] (CWG 1614) + // A function is named by an expression or conversion [...] + // unless it is a pure virtual function and either the expression is not an + // id-expression naming the function with an explicitly qualified name or + // the expression forms a pointer to member bool OdrUse = true; if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(E->getDecl())) if (Method->isVirtual() && diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index 28fcef829ec41..8a9ed2f097165 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -1574,14 +1574,8 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, // Enter a new evaluation context to insulate the lambda from any // cleanups from the enclosing full-expression. - PushExpressionEvaluationContext( - LSI->CallOperator->isConsteval() - ? ExpressionEvaluationContext::ImmediateFunctionContext - : ExpressionEvaluationContext::PotentiallyEvaluated); - ExprEvalContexts.back().InImmediateFunctionContext = - LSI->CallOperator->isConsteval(); - ExprEvalContexts.back().InImmediateEscalatingFunctionContext = - getLangOpts().CPlusPlus20 && LSI->CallOperator->isImmediateEscalating(); + PushExpressionEvaluationContextForFunction( + ExpressionEvaluationContext::PotentiallyEvaluated, LSI->CallOperator); } void Sema::ActOnLambdaError(SourceLocation StartLoc, Scope *CurScope, diff --git a/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp b/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp index 55af13bfc0ef3..20125cc5d4a9c 100644 --- a/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp +++ b/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp @@ -177,4 +177,37 @@ void f() { } } // namespace deduced_return_type_in_discareded_statement +namespace GH140449 { + +template <typename T> +int f() { + T *ptr; + return 0; +} + +template <typename T> +constexpr int g() { + T *ptr; // expected-error{{'ptr' declared as a pointer to a reference of type 'int &'}} + return 0; +} + +template <typename T> +auto h() { + T *ptr; // expected-error{{'ptr' declared as a pointer to a reference of type 'int &'}} + return 0; +} + +void test() { + if constexpr (false) { + int x = f<int &>(); + constexpr int y = g<int &>(); + // expected-error@-1 {{constexpr variable 'y' must be initialized by a constant expression}} \ + // expected-note@-1{{in instantiation of function template specialization}} + int z = h<int &>(); + // expected-note@-1{{in instantiation of function template specialization}} + + } +} +} + #endif `````````` </details> https://github.com/llvm/llvm-project/pull/140576 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
