Skip to content

release/20.x: [Clang] Treat constexpr-unknown value as invalid in EvaluateAsInitializer (#128409) #129836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Mar 5, 2025

Backport 27757fb

Requested by: @frederick-vs-ja

…lizer` (llvm#128409)

It is an alternative to
llvm#127525.
Close llvm#127475.

(cherry picked from commit 27757fb)
@llvmbot
Copy link
Member Author

llvmbot commented Mar 5, 2025

@shafik What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from shafik March 5, 2025 06:27
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 5, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Mar 5, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Mar 5, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 27757fb

Requested by: @frederick-vs-ja


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

3 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+12-2)
  • (modified) clang/lib/CodeGen/CGExprConstant.cpp (+4-1)
  • (added) clang/test/CodeGenCXX/cxx23-p2280r4.cpp (+15)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 0e41e3dbc8a32..a249c3e7d08b7 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3628,8 +3628,6 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   if (AllowConstexprUnknown) {
     if (!Result)
       Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
-    else
-      Result->setConstexprUnknown();
   }
   return true;
 }
@@ -17000,6 +16998,18 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
 
     if (!Info.discardCleanups())
       llvm_unreachable("Unhandled cleanup; missing full expression marker?");
+
+    if (Value.allowConstexprUnknown()) {
+      assert(Value.isLValue() && "Expected an lvalue");
+      auto Base = Value.getLValueBase();
+      const auto *NewVD = Base.dyn_cast<const ValueDecl *>();
+      if (!NewVD)
+        NewVD = VD;
+      Info.FFDiag(getExprLoc(), diag::note_constexpr_var_init_non_constant, 1)
+          << NewVD;
+      NoteLValueLocation(Info, Base);
+      return false;
+    }
   }
 
   return CheckConstantExpression(Info, DeclLoc, DeclTy, Value,
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 655fc3dc954c8..9abbe4b801d56 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1881,8 +1881,11 @@ llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
 
   // Try to emit the initializer.  Note that this can allow some things that
   // are not allowed by tryEmitPrivateForMemory alone.
-  if (APValue *value = D.evaluateValue())
+  if (APValue *value = D.evaluateValue()) {
+    assert(!value->allowConstexprUnknown() &&
+           "Constexpr unknown values are not allowed in CodeGen");
     return tryEmitPrivateForMemory(*value, destType);
+  }
 
   return nullptr;
 }
diff --git a/clang/test/CodeGenCXX/cxx23-p2280r4.cpp b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp
new file mode 100644
index 0000000000000..d5409be451df0
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++23 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++17 %s -emit-llvm -o - | FileCheck %s
+
+extern int& s;
+
+// CHECK: @_Z4testv()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[I:%.*]] = alloca ptr, align {{.*}}
+// CHECK-NEXT: [[X:%.*]] = load ptr, ptr @s, align {{.*}}
+// CHECK-NEXT: store ptr [[X]], ptr [[I]], align {{.*}}
+int& test() {
+  auto &i = s;
+  return i;
+}

@efriedma-quic
Copy link
Collaborator

Closing in favor of #130658, which includes the more comprehensive fix.

@tstellar tstellar moved this from Needs Merge to Done in LLVM Release Status Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants