-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[Clang][Sema] Expression in assumption attribute should be full expression #150814
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
Conversation
@llvm/pr-subscribers-clang Author: Yanzuo Liu (zwuis) ChangesAdd missing Full diff: https://github.com/llvm/llvm-project/pull/150814.diff 5 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 95a0bbf734056..bc0ab7006eecb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -144,6 +144,8 @@ Bug Fixes to C++ Support
- Diagnose binding a reference to ``*nullptr`` during constant evaluation. (#GH48665)
- Suppress ``-Wdeprecated-declarations`` in implicitly generated functions. (#GH147293)
- Fix a crash when deleting a pointer to an incomplete array (#GH150359).
+- Fix an assertion failure when expression in assumption attribute
+ (``[[assume(expr)]]``) creates temporary objects.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 857d46af9ada9..77aa7164d4555 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -795,6 +795,10 @@ ExprResult Sema::BuildCXXAssumeExpr(Expr *Assumption,
if (Res.isInvalid())
return ExprError();
+ Res = ActOnFinishFullExpr(Res.get(), /*DiscardedValue=*/false);
+ if (Res.isInvalid())
+ return ExprError();
+
Assumption = Res.get();
if (Assumption->HasSideEffects(Context))
Diag(Assumption->getBeginLoc(), diag::warn_assume_side_effects)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 20bac0e56b195..d84d0ca1e8296 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2270,11 +2270,6 @@ TemplateInstantiator::TransformCXXAssumeAttr(const CXXAssumeAttr *AA) {
if (!Res.isUsable())
return AA;
- Res = getSema().ActOnFinishFullExpr(Res.get(),
- /*DiscardedValue=*/false);
- if (!Res.isUsable())
- return AA;
-
if (!(Res.get()->getDependence() & ExprDependence::TypeValueInstantiation)) {
Res = getSema().BuildCXXAssumeExpr(Res.get(), AA->getAttrName(),
AA->getRange());
diff --git a/clang/test/Parser/cxx23-assume.cpp b/clang/test/Parser/cxx23-assume.cpp
index 269fb7e599443..375c90820b7ca 100644
--- a/clang/test/Parser/cxx23-assume.cpp
+++ b/clang/test/Parser/cxx23-assume.cpp
@@ -5,7 +5,7 @@ void f(int x, int y) {
[[assume(1)]];
[[assume(1.0)]];
[[assume(1 + 2 == 3)]];
- [[assume(x ? 1 : 2)]];
+ [[assume(x ? 1 : 2)]]; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
[[assume(x && y)]];
[[assume(true)]] [[assume(true)]];
diff --git a/clang/test/SemaCXX/cxx23-assume.cpp b/clang/test/SemaCXX/cxx23-assume.cpp
index 726cb3bff652e..99a82d96d321b 100644
--- a/clang/test/SemaCXX/cxx23-assume.cpp
+++ b/clang/test/SemaCXX/cxx23-assume.cpp
@@ -8,6 +8,14 @@
struct A{};
struct B{ explicit operator bool() { return true; } };
+// This should be the first test case of this file.
+void IsActOnFinishFullExprCalled() {
+ // Do not add other test cases to this function.
+ // Make sure `ActOnFinishFullExpr` is called and creates `ExprWithCleanups`
+ // to avoid assertion failure.
+ [[assume(B{})]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}}
+}
+
template <bool cond>
void f() {
[[assume(cond)]]; // ext-warning {{C++23 extension}}
|
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.
Is there no issue associated with the crash?
I ran into this assertion failure when trying to implement cwg712. I tried to search issues but found nothing. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24252 Here is the relevant piece of the build log for the reference
|
Add missing
ActOnFinishFullExpr
toBuildCXXAssumeExpr
. We did it during template instantiation but forgot non-template case.