-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[clang][bytecode] Create a temporary for discarded CXXBindTemporaryExprs #147303
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
So we run the destructor.
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesSo we run the destructor. Full diff: https://github.com/llvm/llvm-project/pull/147303.diff 3 Files Affected:
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index d1c93e4694667..51c234d0d0471 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2886,7 +2886,20 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
template <class Emitter>
bool Compiler<Emitter>::VisitCXXBindTemporaryExpr(
const CXXBindTemporaryExpr *E) {
- return this->delegate(E->getSubExpr());
+ const Expr *SubExpr = E->getSubExpr();
+
+ if (Initializing)
+ return this->delegate(SubExpr);
+
+ // Make sure we create a temporary even if we're discarding, since that will
+ // make sure we will also call the destructor.
+
+ if (!this->visit(SubExpr))
+ return false;
+
+ if (DiscardResult)
+ return this->emitPopPtr(E);
+ return true;
}
template <class Emitter>
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index 971eb7fd58876..a629ff9569428 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -67,7 +67,8 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
}
if (!Recursing) {
- assert(Stk.empty());
+ // We *can* actually get here with a non-empty stack, since
+ // things like InterpState::noteSideEffect() exist.
C.cleanup();
#ifndef NDEBUG
// Make sure we don't rely on some value being still alive in
diff --git a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
index c5d5427170394..86bed5f14441e 100644
--- a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -3,6 +3,11 @@
// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s "-DNEW=::operator new" "-DDELETE=::operator delete"
// RUN: %clang_cc1 -std=c++2c -verify=expected,cxx26 %s "-DNEW=::operator new" "-DDELETE=::operator delete"
+// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s -DNEW=__builtin_operator_new -DDELETE=__builtin_operator_delete
+// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s "-DNEW=operator new" "-DDELETE=operator delete"
+// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s "-DNEW=::operator new" "-DDELETE=::operator delete"
+// RUN: %clang_cc1 -std=c++2c -verify=expected,cxx26 %s "-DNEW=::operator new" "-DDELETE=::operator delete"
+
constexpr bool alloc_from_user_code() {
void *p = NEW(sizeof(int)); // expected-note {{cannot allocate untyped memory in a constant expression; use 'std::allocator<T>::allocate'}}
DELETE(p);
|
@@ -3,6 +3,11 @@ | |||
// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s "-DNEW=::operator new" "-DDELETE=::operator delete" | |||
// RUN: %clang_cc1 -std=c++2c -verify=expected,cxx26 %s "-DNEW=::operator new" "-DDELETE=::operator delete" | |||
|
|||
// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s -DNEW=__builtin_operator_new -DDELETE=__builtin_operator_delete |
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.
These RUN lines looks like exact duplicates of the original RUN lines?
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.
Yes, sorry. Opened #147590.
These were added in llvm#147303, but the -fexperimental-new-constant-interpreter was missing.
These were added in #147303, but the -fexperimental-new-constant-interpreter was missing.
…p (#147590) These were added in llvm/llvm-project#147303, but the -fexperimental-new-constant-interpreter was missing.
So we run the destructor.