-
Notifications
You must be signed in to change notification settings - Fork 14.6k
release/21.x: [clang] Fix potential constant expression checking with constexpr-unknown. #149402
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
release/21.x: [clang] Fix potential constant expression checking with constexpr-unknown. #149402
Conversation
It looks like I triggered the wrong set of bots by opening the pull request with the wrong base branch. I'm going to close/reopen. |
@llvm/pr-subscribers-clang Author: Eli Friedman (efriedma-quic) ChangesManual backport of 6a60f18 Full diff: https://github.com/llvm/llvm-project/pull/149402.diff 3 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 1b33b6706e204..d7b1173283c57 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4441,7 +4441,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) {
@@ -4478,9 +4479,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 (!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..16f5f823d26c1 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -357,3 +357,29 @@ namespace pointer_comparisons {
static_assert(!f4()); // expected-error {{static assertion expression is not an integral constant expression}} \
// expected-note {{in call to 'f4()'}}
}
+
+namespace GH149188 {
+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..5756bb647ce88 100644
--- a/clang/test/SemaCXX/constexpr-never-constant.cpp
+++ b/clang/test/SemaCXX/constexpr-never-constant.cpp
@@ -24,3 +24,10 @@ constexpr void other_func() {
throw 12;
}
+
+namespace GH149041 {
+ // 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; }
+}
|
Who can review this? |
@AaronBallman can you review or ask someone to do that? This seems to be blocking another fix in #150015 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…nown. 0717657 improved constexpr-unknown diagnostics, but potential constant expression checking broke in the process: we produce diagnostics in more cases. Suppress 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. (cherry picked from commit 6a60f18)
bbf095f
to
ece4440
Compare
@efriedma-quic (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Manual backport of 6a60f18