Skip to content

Conversation

@cor3ntin
Copy link
Contributor

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

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" coroutines C++20 coroutines labels May 19, 2025
@cor3ntin cor3ntin requested review from AaronBallman and erichkeane and removed request for AaronBallman May 19, 2025 17:01
@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-coroutines

Author: cor3ntin (cor3ntin)

Changes

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


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 --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

@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

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


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 --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

@zyn0217
Copy link
Contributor

zyn0217 commented May 19, 2025

Does it fix #115289?

@cor3ntin
Copy link
Contributor Author

Does it fix #115289?

No.

cor3ntin added 2 commits May 20, 2025 07:21
…tiated

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 llvm#140449
PushExpressionEvaluationContextForFunction sets a correct evaluation
context for function, correctly dealing with discarded statements
and immediate escalation.
@cor3ntin cor3ntin force-pushed the corentin/diag_instantiation_in_discarded_stmt branch from 7676b24 to 4c1cc20 Compare May 20, 2025 05:21
@cor3ntin cor3ntin merged commit 5c1db38 into llvm:main May 20, 2025
6 of 10 checks passed
@cor3ntin cor3ntin deleted the corentin/diag_instantiation_in_discarded_stmt branch May 20, 2025 05:22
@BertalanD
Copy link
Member

BertalanD commented May 20, 2025

@cor3ntin fyi, we have been seeing build failures since this commit on code like this (reduced from skia):

void should_compile() {
  if constexpr (false) {
    auto lam = []() { return 0; };
    1 | lam();
  }
}

The error is:

bug.cpp:4:7: error: invalid operands to binary expression ('int' and 'void')
    4 |     1 | lam();
      |     ~ ^ ~~~~~
1 error generated.

@cor3ntin
Copy link
Contributor Author

@BertalanD Investigating

cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request May 21, 2025
Lambda bodies should not be treated as subexpressions of the
enclosing scope.
cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request May 21, 2025
Lambda bodies should not be treated as subexpressions of the
enclosing scope.
cor3ntin added a commit that referenced this pull request May 21, 2025
Lambda bodies should not be treated as subexpressions of the enclosing
scope.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category coroutines C++20 coroutines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clang instantiates function templates inside a discarded statement

5 participants