[PATCH] D153962: [clang] Do not discard cleanups while processing immediate invocation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with two NITs. Thanks a lot for fixing this!



================
Comment at: clang/lib/Sema/SemaExpr.cpp:18187
+  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
----------------
NIT: maybe be a bit more specific about the types of cleanups?
```
// 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.
```


================
Comment at: clang/test/SemaCXX/consteval-cleanup.cpp:1
+// RUN: %clang_cc1 -Wno-unused-value -std=c++20 -ast-dump -verify %s -ast-dump | FileCheck %s
+
----------------
Maybe also add a test with `-fblocks` that checks that only one `ExprWithCleanups` has the block cleanup action attached?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153962/new/

https://reviews.llvm.org/D153962