[clang] fix constexpr-unknown handling of self-references. #132990

efriedma-quic · 2025-03-25T20:51:49Z

Usually, in constant evaluation, references which are local to the evaluation have to be initialized before they're accessed. However, there's one funny special case: the initializer of a reference can refer to itself. This generally ends up being undefined behavior if it's used in an evaluated context, but it isn't otherwise forbidden.

In constant evaluation, this splits into two cases: global variables, and local variables in constexpr functions. This patch handles both of those cases. (Local variables tends to trip other errors in most cases, but if you try hard enough, you can get an accepts-invalid.)

Fixes #131330 .

llvmbot · 2025-03-25T20:52:26Z

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

Usually, in constant evaluation, references which are local to the evaluation have to be initialized before they're accessed. However, there's one funny special case: the initializer of a reference can refer to itself. This generally ends up being undefined behavior if it's used in an evaluated context, but it isn't othewise forbidden.

In constant evaluation, this splits into two cases: global variables, and local variables in constexpr functions. This patch handles both of those cases. (Local variables tends to trip other errors in most cases, but if you try hard enough, you can get an accepts-invalid.)

I'm not sure what's correct for g5(): this patch and gcc accept, but -fexperimental-new-constant-interpreter rejects.

Fixes #131330 .

Full diff: https://github.com/llvm/llvm-project/pull/132990.diff

2 Files Affected:

(modified) clang/lib/AST/ExprConstant.cpp (+30-39)
(modified) clang/test/SemaCXX/constant-expression-p2280r4.cpp (+48-1)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 0165a0a3b0df3..71c8b9a1a5f37 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3485,11 +3485,27 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 
   APValue::LValueBase Base(VD, Frame ? Frame->Index : 0, Version);
 
+  auto CheckUninitReference = [&] {
+    if (!Result->hasValue() && VD->getType()->isReferenceType()) {
+      // We can't use an uninitialized reference directly.
+      if (!AllowConstexprUnknown) {
+        if (!Info.checkingPotentialConstantExpression())
+          Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
+        return false;
+      }
+      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+    }
+    return true;
+  };
+
   // If this is a local variable, dig out its value.
   if (Frame) {
     Result = Frame->getTemporary(VD, Version);
-    if (Result)
+    if (Result) {
+      if (!CheckUninitReference())
+        return false;
       return true;
+    }
 
     if (!isa<ParmVarDecl>(VD)) {
       // Assume variables referenced within a lambda's call operator that were
@@ -3514,6 +3530,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   // in-flight value.
   if (Info.EvaluatingDecl == Base) {
     Result = Info.EvaluatingDeclValue;
+    if (!CheckUninitReference())
+      return false;
+
     return true;
   }
 
@@ -3587,11 +3606,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   // type so we can no longer assume we have an Init.
   // Used to be C++20 [expr.const]p5.12:
   //  ... reference has a preceding initialization and either ...
-  if (Init && !VD->evaluateValue()) {
-    if (AllowConstexprUnknown) {
-      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
-      return true;
-    }
+  if (Init && !VD->evaluateValue() && !AllowConstexprUnknown) {
     Info.FFDiag(E, diag::note_constexpr_var_init_non_constant, 1) << VD;
     NoteLValueLocation(Info, Base);
     return false;
@@ -3629,17 +3644,17 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 
   Result = VD->getEvaluatedValue();
 
-  // C++23 [expr.const]p8
-  // ... For such an object that is not usable in constant expressions, the
-  // dynamic type of the object is constexpr-unknown. For such a reference that
-  // is not usable in constant expressions, the reference is treated as binding
-  // to an unspecified object of the referenced type whose lifetime and that of
-  // all subobjects includes the entire constant evaluation and whose dynamic
-  // type is constexpr-unknown.
-  if (AllowConstexprUnknown) {
-    if (!Result)
+  if (!Result) {
+    if (AllowConstexprUnknown) {
       Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+    } else {
+      return false;
+    }
   }
+
+  if (!CheckUninitReference())
+    return false;
+
   return true;
 }
 
@@ -8931,10 +8946,6 @@ bool LValueExprEvaluator::VisitDeclRefExpr(const DeclRefExpr *E) {
 
 
 bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
-  // C++23 [expr.const]p8 If we have a reference type allow unknown references
-  // and pointers.
-  bool AllowConstexprUnknown =
-      Info.getLangOpts().CPlusPlus23 && VD->getType()->isReferenceType();
   // If we are within a lambda's call operator, check whether the 'VD' referred
   // to within 'E' actually represents a lambda-capture that maps to a
   // data-member/field within the closure object, and if so, evaluate to the
@@ -9001,26 +9012,6 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
   APValue *V;
   if (!evaluateVarDeclInit(Info, E, VD, Frame, Version, V))
     return false;
-  if (!V->hasValue()) {
-    // FIXME: Is it possible for V to be indeterminate here? If so, we should
-    // adjust the diagnostic to say that.
-    // C++23 [expr.const]p8 If we have a variable that is unknown reference
-    // or pointer it may not have a value but still be usable later on so do not
-    // diagnose.
-    if (!Info.checkingPotentialConstantExpression() && !AllowConstexprUnknown)
-      Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
-
-    // C++23 [expr.const]p8 If we have a variable that is unknown reference or
-    // pointer try to recover it from the frame and set the result accordingly.
-    if (VD->getType()->isReferenceType() && AllowConstexprUnknown) {
-      if (Frame) {
-        Result.set({VD, Frame->Index, Version});
-        return true;
-      }
-      return Success(VD);
-    }
-    return false;
-  }
 
   return Success(*V, E);
 }
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index f22430a0e88a2..544372ee4492d 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -std=c++23 -verify=expected,nointerpreter %s
-// RUN: %clang_cc1 -std=c++23 -verify %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++23 -verify=expected,interpreter %s -fexperimental-new-constant-interpreter
 
 using size_t = decltype(sizeof(0));
 
@@ -177,3 +177,50 @@ namespace extern_reference_used_as_unknown {
   int y;
   constinit int& g = (x,y); // expected-warning {{left operand of comma operator has no effect}}
 }
+
+namespace uninit_reference_used {
+  int y;
+  constexpr int &r = r; // expected-error {{must be initialized by a constant expression}} \
+  // nointerpreter-note {{initializer of 'r' is not a constant expression}} \
+  // nointerpreter-note {{declared here}}
+  constexpr int &rr = (rr, y);
+  constexpr int &g() {
+    int &x = x; // expected-warning {{reference 'x' is not yet bound to a value when used within its own initialization}} \
+    // nointerpreter-note {{declared here}} \
+    // interpreter-note {{read of uninitialized object is not allowed in a constant expression}}
+    return x;
+  }
+  constexpr int &gg = g(); // expected-error {{must be initialized by a constant expression}} \
+  // nointerpreter-note {{reference to 'x' is not a constant expression}} \
+  // interpreter-note {{in call to 'g()'}}
+  constexpr int g2() {
+    int &x = x; // expected-warning {{reference 'x' is not yet bound to a value when used within its own initialization}} \
+    // interpreter-note {{read of uninitialized object is not allowed in a constant expression}}
+    return x;
+  }
+  constexpr int gg2 = g2(); // expected-error {{must be initialized by a constant expression}} \
+  // interpreter-note {{in call to 'g2()'}}
+  constexpr int &g3() {
+    int &x = (x,y); // expected-warning{{left operand of comma operator has no effect}} \
+    // expected-warning {{reference 'x' is not yet bound to a value when used within its own initialization}}
+    return x;
+  }
+  constexpr int &gg3 = g3();
+  typedef decltype(sizeof(1)) uintptr_t;
+  constexpr uintptr_t g4() {
+    uintptr_t * &x = x; // expected-warning {{reference 'x' is not yet bound to a value when used within its own initialization}} \
+    // interpreter-note {{read of uninitialized object is not allowed in a constant expression}}
+    *(uintptr_t*)x = 10;
+    return 3;
+  }
+  constexpr uintptr_t gg4 = g4(); // expected-error {{must be initialized by a constant expression}} \
+  // interpreter-note {{in call to 'g4()'}}
+  constexpr int g5() {
+    int &x = x; // expected-warning {{reference 'x' is not yet bound to a value when used within its own initialization}} \
+    // interpreter-note {{read of uninitialized object is not allowed in a constant expression}}
+    return 3;
+  }
+  constexpr uintptr_t gg5 = g5(); // interpreter-error {{must be initialized by a constant expression}} \
+  // interpreter-note {{in call to 'g5()'}}
+
+}

github-actions · 2025-03-25T20:55:23Z

✅ With the latest revision this PR passed the C/C++ code formatter.

cor3ntin

Thanks.
I think the direction makes sense

Do you intend to backport this change to 20?

cor3ntin · 2025-03-26T08:54:22Z

clang/lib/AST/ExprConstant.cpp

+  if (!Result) {
+    if (AllowConstexprUnknown) {
      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+    } else {
+      return false;
+    }


Suggested change

if (!Result) {

if (AllowConstexprUnknown) {

Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);

} else {

return false;

}

if (!Result) {

if (AllowConstexprUnknown)

Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);

else

return false;

cor3ntin · 2025-03-26T08:57:17Z

clang/lib/AST/ExprConstant.cpp

+      if (!CheckUninitReference())
+        return false;


return CheckUninitReference();

cor3ntin · 2025-03-26T08:57:35Z

clang/lib/AST/ExprConstant.cpp

+  auto CheckUninitReference = [&] {
+    if (!Result->hasValue() && VD->getType()->isReferenceType()) {
+      // We can't use an uninitialized reference directly.
+      if (!AllowConstexprUnknown) {
+        if (!Info.checkingPotentialConstantExpression())
+          Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
+        return false;
+      }
+      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+    }
+    return true;
+  };
+


I think this could use a comment

cor3ntin · 2025-03-26T09:02:35Z

clang/test/SemaCXX/constant-expression-p2280r4.cpp

+  constexpr uintptr_t gg5 = g5(); // interpreter-error {{must be initialized by a constant expression}} \
+  // interpreter-note {{in call to 'g5()'}}


g5() looks valid to me - in the initializer x refer to a reference whose lifetime has not yet started, and therefore it is constexpr-unknown and should not be an error
@t3nsor

The relevant standard language is, I think, "all id-expressions and uses of *this that refer to an object or reference whose lifetime did not begin with the evaluation of E[...]". For a local variable inside a constexpr function, I think it does "begin with the evaluation of E", even if its lifetime hasn't yet started, because it's part of the constexpr call frame. Therefore it's not allowed. I might be missing something, though.

There's also a sort of practical argument: it's weird to say that int &x = x; in a constexpr function is well-defined during constant evaluation, but has undefined behavior at runtime.

That said, the compiler's state is internally consistent either way.

I think it would be extremely strange if the intent of the standard were to not allow this case to be diagnosed. Going by the literal wording, yes, you have an id-expression that refers to a reference whose lifetime didn't begin within the constant evaluation (because it hasn't begun at all), which makes it an unknown reference, but in reality we know exactly what's happening here. It seems like the standard may have a defect and the rule for references really should be that the reference is unknown only if its initialization didn't begin within the evaluation of E.

(I mailed core)

based on cwg453 this is UB and we should not accept it.

cor3ntin · 2025-03-26T09:03:05Z

clang/lib/AST/ExprConstant.cpp

-  // C++23 [expr.const]p8
-  // ... For such an object that is not usable in constant expressions, the
-  // dynamic type of the object is constexpr-unknown. For such a reference that
-  // is not usable in constant expressions, the reference is treated as binding
-  // to an unspecified object of the referenced type whose lifetime and that of
-  // all subobjects includes the entire constant evaluation and whose dynamic
-  // type is constexpr-unknown.


Why remove the comment?

The comment didn't make sense at that location in the code, given the refactoring. I could stick something into CheckUninitReference, I guess.

Please move this up to CheckUninitReference

efriedma-quic · 2025-03-26T19:13:09Z

Do you intend to backport this change to 20?

The weird behavior only shows up if you refer to a reference variable inside its own initializer, as far as I can tell. Maybe not worth backporting, even if it is a regression, because people shouldn't be writing code like that in the first place.

cor3ntin · 2025-03-26T20:01:18Z

Do you intend to backport this change to 20?

The weird behavior only shows up if you refer to a reference variable inside its own initializer, as far as I can tell. Maybe not worth backporting, even if it is a regression, because people shouldn't be writing code like that in the first place.

Fair enough, in that case, can you add a changelog entry? Thanks!

efriedma-quic

Addressed review comments. We now reject g3() and g5(), the cases that involved using a local variable in a constexpr function in its own initializer.

shafik

LGTM any objections @cor3ntin @t3nsor?

efriedma-quic · 2025-04-28T23:31:49Z

Ping

t3nsor · 2025-04-29T01:49:13Z

No opinions about the code, but on the reflector we did agree that rejecting those initialization cases was the right thing to do.

cor3ntin

LGTM modulo nit

cor3ntin · 2025-05-16T09:53:45Z

clang/lib/AST/ExprConstant.cpp

+  if (!Result) {
+    if (AllowConstexprUnknown) {
      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+    } else {
+      return false;
+    }


Usually, in constant evaluation, references which are local to the evaluation have to be initialized before they're accessed. However, there's one funny special case: the initializer of a reference can refer to itself. This generally ends up being undefined behavior if it's used in an evaluated context, but it isn't othewise forbidden. In constant evaluation, this splits into two cases: global variables, and local variables in constexpr functions. This patch handles both of those cases. (Local variables tends to trip other errors in most cases, but if you try hard enough, you can get an accepts-invalid.) Fixes llvm#131330 .

efriedma-quic requested review from cor3ntin and tbaederr March 25, 2025 20:51

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 25, 2025

efriedma-quic requested review from frederick-vs-ja and shafik March 25, 2025 20:52

efriedma-quic force-pushed the constexpr-unknown-uninit branch from f5195b8 to 2873bb1 Compare March 25, 2025 20:59

cor3ntin reviewed Mar 26, 2025

View reviewed changes

efriedma-quic commented Mar 28, 2025

View reviewed changes

shafik approved these changes Apr 8, 2025

View reviewed changes

efriedma-quic force-pushed the constexpr-unknown-uninit branch 3 times, most recently from 99df9ff to 51bd288 Compare May 16, 2025 00:31

cor3ntin approved these changes May 16, 2025

View reviewed changes

efriedma-quic force-pushed the constexpr-unknown-uninit branch from 51bd288 to 3d02937 Compare May 16, 2025 18:25

efriedma-quic merged commit 021443c into llvm:main May 19, 2025
12 checks passed

		constexpr uintptr_t gg5 = g5(); // interpreter-error {{must be initialized by a constant expression}} \
		// interpreter-note {{in call to 'g5()'}}

[clang] fix constexpr-unknown handling of self-references. #132990

[clang] fix constexpr-unknown handling of self-references. #132990

Uh oh!

Conversation

efriedma-quic commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 25, 2025

Uh oh!

github-actions bot commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic commented Mar 26, 2025

Uh oh!

cor3ntin commented Mar 26, 2025

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

efriedma-quic commented Apr 28, 2025

Uh oh!

t3nsor commented Apr 29, 2025

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

efriedma-quic commented Mar 25, 2025 •

edited

Loading

github-actions bot commented Mar 25, 2025 •

edited

Loading