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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
