[clang] [Clang] Functions called in discarded statements should not be instantiated (PR #140576)
via cfe-commits
cfe-commits at lists.llvm.org
Mon May 19 10:01:13 PDT 2025
https://github.com/cor3ntin created https://github.com/llvm/llvm-project/pull/140576
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
>From 65f66115fcf3ebb2a5dfaeed65168fe8ee8ff64c Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Mon, 19 May 2025 17:18:10 +0200
Subject: [PATCH 1/2] [Clang] Functions called in discarded statements should
not be instantiated
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 #140449
---
clang/docs/ReleaseNotes.rst | 1 +
clang/include/clang/Sema/Sema.h | 5 ++-
clang/lib/Sema/SemaCoroutine.cpp | 3 ++
clang/lib/Sema/SemaDecl.cpp | 3 ++
clang/lib/Sema/SemaExpr.cpp | 44 +++++++------------
.../CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp | 33 ++++++++++++++
6 files changed, 60 insertions(+), 29 deletions(-)
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/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ec67087aeea4..11b2eb48d8e91 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);
}
};
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 279e4c77d04aa..e17a8dbfaf635 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -699,6 +699,9 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
// Ignore previous expr evaluation contexts.
EnterExpressionEvaluationContext PotentiallyEvaluated(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+
+ ExprEvalContexts.back().InDiscardedStatement = false;
+
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..bbfa55fc71d23 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15885,6 +15885,9 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
+ // A function that is not a lambda is never in a discarded statement
+ ExprEvalContexts.back().InDiscardedStatement = false;
+
// Check for defining attributes before the check for redefinition.
if (const auto *Attr = FD->getAttr<AliasAttr>()) {
Diag(Attr->getLocation(), diag::err_alias_is_definition) << FD << 0;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 91e63c7cb8677..8e50c6aee705e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18361,35 +18361,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;
+ const Sema::ExpressionEvaluationContextRecord &Context =
+ SemaRef.currentEvaluationContext();
- 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;
-
- 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 +20344,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/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 at -1 {{constexpr variable 'y' must be initialized by a constant expression}} \
+ // expected-note at -1{{in instantiation of function template specialization}}
+ int z = h<int &>();
+ // expected-note at -1{{in instantiation of function template specialization}}
+
+ }
+}
+}
+
#endif
>From 7676b243aef90fdb91458230a823b51fec062c4b Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Mon, 19 May 2025 18:38:32 +0200
Subject: [PATCH 2/2] Refactor PushExpressionEvaluationContext to avoid
duplication
PushExpressionEvaluationContextForFunction sets a correct evaluation
context for function, correctly dealing with discarded statements
and immediate escalation.
---
.../Sema/EnterExpressionEvaluationContext.h | 16 ++++++++
clang/include/clang/Sema/Sema.h | 24 ++++-------
clang/lib/Sema/SemaCoroutine.cpp | 7 ++--
clang/lib/Sema/SemaDecl.cpp | 23 +----------
clang/lib/Sema/SemaExpr.cpp | 41 +++++++++++++++++++
clang/lib/Sema/SemaLambda.cpp | 10 +----
6 files changed, 72 insertions(+), 49 deletions(-)
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 11b2eb48d8e91..9af3387b533c0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6916,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,
@@ -13372,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 e17a8dbfaf635..425b32e53a7b7 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -697,10 +697,9 @@ 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);
-
- ExprEvalContexts.back().InDiscardedStatement = false;
+ EnterExpressionEvaluationContextForFunction PotentiallyEvaluated(
+ *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+ dyn_cast_or_null<FunctionDecl>(CurContext));
if (!checkCoroutineContext(*this, KWLoc, Keyword))
return false;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bbfa55fc71d23..02a0ed2172bd7 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15866,27 +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();
-
- // A function that is not a lambda is never in a discarded statement
- ExprEvalContexts.back().InDiscardedStatement = false;
+ 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 8e50c6aee705e..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) {
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,
More information about the cfe-commits
mailing list