diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -549,6 +549,10 @@ (`#48512 `_). - Fixed a failing assertion when parsing incomplete destructor. (`#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 `_). Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -18183,7 +18183,24 @@ return E; } - E = MaybeCreateExprWithCleanups(E); + if (Cleanup.exprNeedsCleanups()) { + // 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 so + // create ExprWithCleanups without using MaybeCreateExprWithCleanups as it + // may discard cleanups for outer expression too early. + + // Note that ExprWithCleanups created here must always have empty cleanup + // objects: + // - compound literals do not create cleanup objects in C++ and immediate + // invocations are C++-only. + // - blocks are not allowed inside constant expressions and compiler will + // issue an error if they appear there. Hence, in correct code any cleanup + // objects created inside current evaluation context must be outside the + // immediate invocation. + E = ExprWithCleanups::Create(getASTContext(), E.get(), + Cleanup.cleanupsHaveSideEffects(), {}); + } ConstantExpr *Res = ConstantExpr::Create( getASTContext(), E.get(), diff --git a/clang/test/CodeGenCXX/consteval-cleanup.cpp b/clang/test/CodeGenCXX/consteval-cleanup.cpp new file mode 100644 --- /dev/null +++ b/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 +} diff --git a/clang/test/SemaCXX/consteval-cleanup.cpp b/clang/test/SemaCXX/consteval-cleanup.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/consteval-cleanup.cpp @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -fblocks -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); } + const int& get() const { + return *this->data; + } + ~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 +} + +void foobar() { + A a(1); + for (; ^{ auto ptr = &a.get(); }(), P(), false;); + // CHECK: ExprWithCleanups + // CHECK-NEXT: cleanup Block + // CHECK-NEXT: BinaryOperator {{.*}} 'bool' ',' + // CHECK-NEXT: BinaryOperator {{.*}} 'P':'P' ',' + // CHECK-NEXT: CallExpr + // CHECK-NEXT: BlockExpr + // CHECK: ConstantExpr {{.*}} 'P':'P' + // CHECK-NEXT: value: + // CHECK-NEXT: ExprWithCleanups + // CHECK-NOT: cleanup Block +} + +struct B { + int *p = new int(38); + consteval int get() { return *p; } + constexpr ~B() { delete p; } +}; + +void bar() { + // CHECK: bar + // CHECK: ExprWithCleanups + // CHECK: ConstantExpr + // CHECK-NEXT: value: + // CHECK-NEXT: ExprWithCleanups + int k = B().get(); +}