[clang] Fix potential constant expression checking with constexpr-unknown. #149227
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nown. 0717657 improved constexpr-unkown diagnostics, but potential constant expression checking broke in the process: we produce diagnostics in more cases. Supress the diagnostics as appropriate. This fix affects -Winvalid-constexpr and the enable_if attribute. (The -Winvalid-constexpr diagnostic isn't really important right now, but it will become important if we allow constexpr-unknown with pre-C++23 standards.) Fixes llvm#149041. Fixes llvm#149188.
efriedma-quic
requested review from
cor3ntin,
shafik,
frederick-vs-ja and
Sirraide
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Eli Friedman (efriedma-quic) Changes0717657 improved constexpr-unkown diagnostics, but potential constant expression checking broke in the process: we produce diagnostics in more cases. Supress the diagnostics as appropriate. This fix affects -Winvalid-constexpr and the enable_if attribute. (The -Winvalid-constexpr diagnostic isn't really important right now, but it will become important if we allow constexpr-unknown with pre-C++23 standards.) Fixes #149041. Fixes #149188. Full diff: https://github.com/llvm/llvm-project/pull/149227.diff 3 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 767cc4c3b19eb..8797eaddd0e18 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4450,7 +4450,8 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
}
} else if (!IsAccess) {
return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
- } else if (IsConstant && Info.checkingPotentialConstantExpression() &&
+ } else if ((IsConstant || BaseType->isReferenceType()) &&
+ Info.checkingPotentialConstantExpression() &&
BaseType->isLiteralType(Info.Ctx) && !VD->hasDefinition()) {
// This variable might end up being constexpr. Don't diagnose it yet.
} else if (IsConstant) {
@@ -4491,9 +4492,11 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
// a null BaseVal. Any constexpr-unknown variable seen here is an error:
// we can't access a constexpr-unknown object.
if (AK != clang::AK_Dereference && !BaseVal) {
- Info.FFDiag(E, diag::note_constexpr_access_unknown_variable, 1)
- << AK << VD;
- Info.Note(VD->getLocation(), diag::note_declared_at);
+ if (!Info.checkingPotentialConstantExpression()) {
+ Info.FFDiag(E, diag::note_constexpr_access_unknown_variable, 1)
+ << AK << VD;
+ Info.Note(VD->getLocation(), diag::note_declared_at);
+ }
return CompleteObject();
}
} else if (DynamicAllocLValue DA = LVal.Base.dyn_cast<DynamicAllocLValue>()) {
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index 03fea91169787..979d29476c119 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -357,3 +357,27 @@ namespace pointer_comparisons {
static_assert(!f4()); // expected-error {{static assertion expression is not an integral constant expression}} \
// expected-note {{in call to 'f4()'}}
}
+
+namespace enable_if_1 {
+ template <__SIZE_TYPE__ N>
+ constexpr void foo(const char (&Str)[N])
+ __attribute((enable_if(__builtin_strlen(Str), ""))) {}
+
+ void x() {
+ foo("1234");
+ }
+}
+
+namespace enable_if_2 {
+ constexpr const char (&f())[];
+ extern const char (&Str)[];
+ constexpr int foo()
+ __attribute((enable_if(__builtin_strlen(Str), "")))
+ {return __builtin_strlen(Str);}
+
+ constexpr const char (&f())[] {return "a";}
+ constexpr const char (&Str)[] = f();
+ void x() {
+ constexpr int x = foo();
+ }
+}
diff --git a/clang/test/SemaCXX/constexpr-never-constant.cpp b/clang/test/SemaCXX/constexpr-never-constant.cpp
index 307810ee263dd..ca6dff22275d5 100644
--- a/clang/test/SemaCXX/constexpr-never-constant.cpp
+++ b/clang/test/SemaCXX/constexpr-never-constant.cpp
@@ -24,3 +24,8 @@ constexpr void other_func() {
throw 12;
}
+
+// Make sure these don't trigger the diagnostic.
+extern const bool& b;
+constexpr bool fun1() { return b; }
+constexpr bool fun2(const bool& b) { return b; }
|
cor3ntin
approved these changes
Jul 17, 2025
Comment on lines
+361
to
+383
| namespace enable_if_1 { | ||
| template <__SIZE_TYPE__ N> | ||
| constexpr void foo(const char (&Str)[N]) | ||
| __attribute((enable_if(__builtin_strlen(Str), ""))) {} | ||
|
|
||
| void x() { | ||
| foo("1234"); | ||
| } | ||
| } | ||
|
|
||
| namespace enable_if_2 { | ||
| constexpr const char (&f())[]; | ||
| extern const char (&Str)[]; | ||
| constexpr int foo() | ||
| __attribute((enable_if(__builtin_strlen(Str), ""))) | ||
| {return __builtin_strlen(Str);} | ||
|
|
||
| constexpr const char (&f())[] {return "a";} | ||
| constexpr const char (&Str)[] = f(); | ||
| void x() { | ||
| constexpr int x = foo(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you reference the issue (either a comment or in the namespace name)?
There was a problem hiding this comment.
We normally use the namespace name, I think we should stick w/ that.
| // Make sure these don't trigger the diagnostic. | ||
| extern const bool& b; | ||
| constexpr bool fun1() { return b; } | ||
| constexpr bool fun2(const bool& b) { return b; } |
There was a problem hiding this comment.
Can you reference the issue (either a comment or in the namespace name)?
github-project-automation
bot
moved this from Needs Triage
to Done
in LLVM Release Status
|
/cherry-pick 6a60f18 |
|
Failed to cherry-pick: 6a60f18 https://github.com/llvm/llvm-project/actions/runs/16355033735 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
shafik
reviewed
Jul 17, 2025
Comment on lines
+361
to
+383
| namespace enable_if_1 { | ||
| template <__SIZE_TYPE__ N> | ||
| constexpr void foo(const char (&Str)[N]) | ||
| __attribute((enable_if(__builtin_strlen(Str), ""))) {} | ||
|
|
||
| void x() { | ||
| foo("1234"); | ||
| } | ||
| } | ||
|
|
||
| namespace enable_if_2 { | ||
| constexpr const char (&f())[]; | ||
| extern const char (&Str)[]; | ||
| constexpr int foo() | ||
| __attribute((enable_if(__builtin_strlen(Str), ""))) | ||
| {return __builtin_strlen(Str);} | ||
|
|
||
| constexpr const char (&f())[] {return "a";} | ||
| constexpr const char (&Str)[] = f(); | ||
| void x() { | ||
| constexpr int x = foo(); | ||
| } | ||
| } |
There was a problem hiding this comment.
We normally use the namespace name, I think we should stick w/ that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
0717657 improved constexpr-unkown diagnostics, but potential constant expression checking broke in the process: we produce diagnostics in more cases. Supress the diagnostics as appropriate.
This fix affects -Winvalid-constexpr and the enable_if attribute. (The -Winvalid-constexpr diagnostic isn't really important right now, but it will become important if we allow constexpr-unknown with pre-C++23 standards.)
Fixes #149041. Fixes #149188.