From 9010db1b84ef81d23266b5c57569beadd7967ddb Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Wed, 5 Mar 2025 14:01:24 +0800 Subject: [PATCH 1/3] [Clang] Treat constexpr-unknown value as invalid in `EvaluateAsInitializer` (#128409) It is an alternative to https://github.com/llvm/llvm-project/pull/127525. Close https://github.com/llvm/llvm-project/issues/127475. --- clang/lib/AST/ExprConstant.cpp | 14 ++++++++++++-- clang/lib/CodeGen/CGExprConstant.cpp | 5 ++++- clang/test/CodeGenCXX/cxx23-p2280r4.cpp | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 clang/test/CodeGenCXX/cxx23-p2280r4.cpp 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(); + 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; +} From fbb2a7e74d918752aed9442ac8883ace8b636c50 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sun, 9 Mar 2025 18:38:55 -0700 Subject: [PATCH 2/3] [clang] Reject constexpr-unknown values as constant expressions more consistently (#129952) Perform the check for constexpr-unknown values in the same place we perform checks for other values which don't count as constant expressions. While I'm here, also fix a rejects-valid with a reference that doesn't have an initializer. This diagnostic was also covering up some of the bugs here. The existing behavior with -fexperimental-new-constant-interpreter seems to be correct, but the diagnostics are slightly different; it would be helpful if someone could check on that as a followup. Followup to #128409. Fixes #129844. Fixes #129845. --- clang/lib/AST/ExprConstant.cpp | 25 +++++++-------- clang/test/CodeGenCXX/cxx23-p2280r4.cpp | 15 ++++++++- .../SemaCXX/constant-expression-cxx11.cpp | 4 +-- .../SemaCXX/constant-expression-p2280r4.cpp | 32 +++++++++++++++++-- 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index a249c3e7d08b7..5aae78dd2fee7 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -2419,6 +2419,16 @@ static bool CheckLValueConstantExpression(EvalInfo &Info, SourceLocation Loc, LVal.getLValueCallIndex() == 0) && "have call index for global lvalue"); + if (LVal.allowConstexprUnknown()) { + if (BaseVD) { + Info.FFDiag(Loc, diag::note_constexpr_var_init_non_constant, 1) << BaseVD; + NoteLValueLocation(Info, Base); + } else { + Info.FFDiag(Loc); + } + return false; + } + if (Base.is()) { Info.FFDiag(Loc, diag::note_constexpr_dynamic_alloc) << IsReferenceType << !Designator.Entries.empty(); @@ -3597,7 +3607,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, // expressions here; doing so would regress diagnostics for things like // reading from a volatile constexpr variable. if ((Info.getLangOpts().CPlusPlus && !VD->hasConstantInitialization() && - VD->mightBeUsableInConstantExpressions(Info.Ctx)) || + VD->mightBeUsableInConstantExpressions(Info.Ctx) && + !AllowConstexprUnknown) || ((Info.getLangOpts().CPlusPlus || Info.getLangOpts().OpenCL) && !Info.getLangOpts().CPlusPlus11 && !VD->hasICEInitializer(Info.Ctx))) { if (Init) { @@ -16998,18 +17009,6 @@ 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(); - 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/test/CodeGenCXX/cxx23-p2280r4.cpp b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp index d5409be451df0..b62c68c66f0fa 100644 --- a/clang/test/CodeGenCXX/cxx23-p2280r4.cpp +++ b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp @@ -4,7 +4,7 @@ extern int& s; -// CHECK: @_Z4testv() +// CHECK-LABEL: @_Z4testv() // CHECK-NEXT: entry: // CHECK-NEXT: [[I:%.*]] = alloca ptr, align {{.*}} // CHECK-NEXT: [[X:%.*]] = load ptr, ptr @s, align {{.*}} @@ -13,3 +13,16 @@ int& test() { auto &i = s; return i; } + +// CHECK-LABEL: @_Z1fv( +// CHECK: [[X1:%.*]] = load ptr, ptr @x, align 8 +// CHECK-NEXT: store ptr [[X1]] +// CHECK: [[X2:%.*]] = load ptr, ptr @x, align 8 +// CHECK-NEXT: store ptr [[X2]] +// CHECK: [[X3:%.*]] = load ptr, ptr @x, align 8 +// CHECK-NEXT: store ptr [[X3]] +int &ff(); +int &x = ff(); +struct A { int& x; }; +struct B { A x[20]; }; +B f() { return {x,x,x,x,x,x,x,x,x,x,x,x,x,x,x,x,x,x,x,x}; } diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp index 76e2f81947051..c35f3a5632a05 100644 --- a/clang/test/SemaCXX/constant-expression-cxx11.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp @@ -1472,8 +1472,8 @@ namespace ConvertedConstantExpr { enum class E { em = m, en = n, // expected-error {{enumerator value is not a constant expression}} cxx11_20-note {{initializer of 'n' is unknown}} - eo = (m + // pre-cxx23-error {{not a constant expression}} - n // cxx11_20-note {{initializer of 'n' is unknown}} cxx23-error {{not a constant expression}} + eo = (m + // expected-error {{not a constant expression}} + n // cxx11_20-note {{initializer of 'n' is unknown}} ), eq = reinterpret_cast((int*)0) // expected-error {{not a constant expression}} expected-note {{reinterpret_cast}} }; diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp index 8648350b397e0..6c9a87267109c 100644 --- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp +++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp @@ -1,4 +1,7 @@ -// RUN: %clang_cc1 -std=c++23 -verify %s +// RUN: %clang_cc1 -std=c++23 -verify=expected,nointerpreter %s +// (Run line removed for backport to 20.x, so we don't need to backport +// fexperimental-new-constant-interpreter changes) +// UN: %clang_cc1 -std=c++23 -verify %s -fexperimental-new-constant-interpreter using size_t = decltype(sizeof(0)); @@ -38,8 +41,8 @@ void splash(Swim& swam) { static_assert(swam.phelps() == 28); // ok static_assert((&swam)->phelps() == 28); // ok Swim* pswam = &swam; // expected-note {{declared here}} - static_assert(pswam->phelps() == 28); // expected-error {{static assertion expression is not an integral constant expression}} - // expected-note@-1 {{read of non-constexpr variable 'pswam' is not allowed in a constant expression}} + static_assert(pswam->phelps() == 28); // expected-error {{static assertion expression is not an integral constant expression}} \ + // expected-note {{read of non-constexpr variable 'pswam' is not allowed in a constant expression}} static_assert(how_many(swam) == 28); // ok static_assert(Swim().lochte() == 12); // ok static_assert(swam.lochte() == 12); // expected-error {{static assertion expression is not an integral constant expression}} @@ -153,3 +156,26 @@ int g() { static_assert(f(arr) == 5); } } + +namespace GH128409 { + int &ff(); + int &x = ff(); // nointerpreter-note {{declared here}} + constinit int &z = x; // expected-error {{variable does not have a constant initializer}} \ + // expected-note {{required by 'constinit' specifier here}} \ + // nointerpreter-note {{initializer of 'x' is not a constant expression}} +} + +namespace GH129845 { + int &ff(); + int &x = ff(); // nointerpreter-note {{declared here}} + struct A { int& x; }; + constexpr A g = {x}; // expected-error {{constexpr variable 'g' must be initialized by a constant expression}} \ + // nointerpreter-note {{initializer of 'x' is not a constant expression}} + const A* gg = &g; +} + +namespace extern_reference_used_as_unknown { + extern int &x; + int y; + constinit int& g = (x,y); // expected-warning {{left operand of comma operator has no effect}} +} From 72c4a3f419f4ec5397a32859a20b112e225df4ea Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 10 Mar 2025 13:45:44 +0000 Subject: [PATCH 3/3] [clang][test] Don't require specific alignment in test case (#130589) https://github.com/llvm/llvm-project/pull/129952 / 42d49a77241df73a17cb442973702fc460e7fb90 added this test which is failing on 32-bit ARM because the alignment chosen is 4 not 8. Which would make sense if this is a 32/64 bit difference https://lab.llvm.org/buildbot/#/builders/154/builds/13059 ``` :34:30: note: scanning from here define dso_local void @_Z1fv(ptr dead_on_unwind noalias writable sret(%struct.B) align 4 %agg.result) #0 { ^ :38:2: note: possible intended match here %0 = load ptr, ptr @x, align 4 ^ ``` The other test does not check alignment, so I'm assuming that it is not important here. --- clang/test/CodeGenCXX/cxx23-p2280r4.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/CodeGenCXX/cxx23-p2280r4.cpp b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp index b62c68c66f0fa..53b00695d9d6d 100644 --- a/clang/test/CodeGenCXX/cxx23-p2280r4.cpp +++ b/clang/test/CodeGenCXX/cxx23-p2280r4.cpp @@ -15,11 +15,11 @@ int& test() { } // CHECK-LABEL: @_Z1fv( -// CHECK: [[X1:%.*]] = load ptr, ptr @x, align 8 +// CHECK: [[X1:%.*]] = load ptr, ptr @x, align {{.*}} // CHECK-NEXT: store ptr [[X1]] -// CHECK: [[X2:%.*]] = load ptr, ptr @x, align 8 +// CHECK: [[X2:%.*]] = load ptr, ptr @x, align {{.*}} // CHECK-NEXT: store ptr [[X2]] -// CHECK: [[X3:%.*]] = load ptr, ptr @x, align 8 +// CHECK: [[X3:%.*]] = load ptr, ptr @x, align {{.*}} // CHECK-NEXT: store ptr [[X3]] int &ff(); int &x = ff();