Fznamznon updated this revision to Diff 535419.
Fznamznon added a comment.
Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153962/new/
https://reviews.llvm.org/D153962
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenCXX/consteval-cleanup.cpp
clang/test/SemaCXX/consteval-cleanup.cpp
Index: clang/test/SemaCXX/consteval-cleanup.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/consteval-cleanup.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -Wno-unused-value -std=c++20 -ast-dump -verify %s -ast-dump | FileCheck %s
+
+// expected-no-diagnostics
+
+struct P {
+ consteval P() {}
+};
+
+struct A {
+ A(int v) { this->data = new int(v); }
+ ~A() { delete data; }
+private:
+ int *data;
+};
+
+void foo() {
+ for (;A(1), P(), false;);
+ // CHECK: foo
+ // CHECK: ExprWithCleanups
+ // CHECK-NEXT: BinaryOperator {{.*}} 'bool' ','
+ // CHECK-NEXT: BinaryOperator {{.*}} 'P':'P' ','
+ // CHECK-NEXT: CXXFunctionalCastExpr {{.*}} 'A':'A'
+ // CHECK-NEXT: CXXBindTemporaryExpr {{.*}} 'A':'A'
+ // CHECK-NEXT: CXXConstructExpr {{.*}} 'A':'A'
+ // CHECK: ConstantExpr {{.*}} 'P':'P'
+ // CHECK-NEXT: value:
+ // CHECK-NEXT: ExprWithCleanups
+}
+
+struct B {
+ int *p = new int(38);
+ consteval int get() { return *p; }
+ constexpr ~B() { delete p; }
+};
+
+void bar() {
+ // CHECK: bar
+ // CHECK: ConstantExpr
+ // CHECK-NEXT: value:
+ // CHECK-NEXT: ExprWithCleanups
+ int k = B().get();
+}
Index: clang/test/CodeGenCXX/consteval-cleanup.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/consteval-cleanup.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+struct P {
+ consteval P() {}
+};
+
+struct A {
+ A(int v) { this->data = new int(v); }
+ ~A() { delete data; }
+private:
+ int *data;
+};
+
+void foo() {
+ for (;A(1), P(), false;);
+ // CHECK: foo
+ // CHECK: for.cond:
+ // CHECK: call void @_ZN1AC1Ei
+ // CHECK: call void @_ZN1AD1Ev
+ // CHECK: for.body
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7346,15 +7346,16 @@
return Bind;
}
-ExprResult
-Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr) {
+ExprResult Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr,
+ bool IsImmediateInvocation) {
if (SubExpr.isInvalid())
return ExprError();
- return MaybeCreateExprWithCleanups(SubExpr.get());
+ return MaybeCreateExprWithCleanups(SubExpr.get(), IsImmediateInvocation);
}
-Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) {
+Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr,
+ bool IsImmediateInvocation) {
assert(SubExpr && "subexpression can't be null!");
CleanupVarDeclMarking();
@@ -7371,8 +7372,22 @@
auto *E = ExprWithCleanups::Create(
Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups);
- DiscardCleanupsInEvaluationContext();
+ if (IsImmediateInvocation) {
+ // Do not discard cleanups in case we're processing an immediate invocation
+ // since an immediate invocation is a full expression itself - it requires
+ // an additional ExprWithCleanups node, but it can participate to a bigger
+ // full expression which actually requires cleanups to be run after.
+
+ // ExprCleanupObjects contain BlockDecl or CompoundLiteralExpr nodes which
+ // can't end up attached to a ExprWithCleanups created for an immediate
+ // invocation, because BlockDecl is ObjC construct and compound literals
+ // in C++ are just temporaries, so it is ok to not care about them.
+ assert(LangOpts.CPlusPlus && "Immediate invocation outside of C++?");
+ return E;
+ }
+
+ DiscardCleanupsInEvaluationContext();
return E;
}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18183,7 +18183,7 @@
return E;
}
- E = MaybeCreateExprWithCleanups(E);
+ E = MaybeCreateExprWithCleanups(E, /*IsImmediateInvocation=*/true);
ConstantExpr *Res = ConstantExpr::Create(
getASTContext(), E.get(),
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6911,9 +6911,11 @@
/// MaybeCreateExprWithCleanups - If the current full-expression
/// requires any cleanups, surround it with a ExprWithCleanups node.
/// Otherwise, just returns the passed-in expression.
- Expr *MaybeCreateExprWithCleanups(Expr *SubExpr);
+ Expr *MaybeCreateExprWithCleanups(Expr *SubExpr,
+ bool IsImmediateInvocation = false);
Stmt *MaybeCreateStmtWithCleanups(Stmt *SubStmt);
- ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr);
+ ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr,
+ bool IsImmediateInvocation = false);
MaterializeTemporaryExpr *
CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -549,6 +549,10 @@
(`#48512 <https://github.com/llvm/llvm-project/issues/48512>`_).
- Fixed a failing assertion when parsing incomplete destructor.
(`#63503 <https://github.com/llvm/llvm-project/issues/63503>`_)
+- Fix missing destructor calls and therefore memory leaks in generated code
+ when an immediate invocation appears as a part of an expression that produces
+ temporaries.
+ (`#60709 <https://github.com/llvm/llvm-project/issues/60709>`_).
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
