Fznamznon added inline comments.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:7374
auto *E = ExprWithCleanups::Create(
Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups);
----------------
ilya-biryukov wrote:
> Because we may potentially pass some cleanups here (which are outside
> `ConstantExpr`) and won't discard them later, we end up adding the unrelated
> blocks to this cleanup expression.
> Here's an example. If we start with this code:
> ```cpp
> 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() {
> A a(1);
> for (; ^{ ptr = &a.get(); }(), P(), false;);
> }
> ```
> And run `clang++ -std=c++20 -fblocks -Xclang=-ast-dump`, we'll get:
> (notice same Block in two cleanups)
> ```
> `-ForStmt 0x5592888b1318 <line:17:3, col:46>
> |-<<<NULL>>>
> |-<<<NULL>>>
> |-ExprWithCleanups 0x5592888b12f0 <col:10, col:39> 'bool'
> | |-cleanup Block 0x5592888ad728
> | `-BinaryOperator 0x5592888b12d0 <col:10, col:39> 'bool' ','
> | |-BinaryOperator 0x5592888b12a0 <col:10, col:36> 'P':'P' ','
> | | |-CallExpr 0x5592888adb48 <col:10, col:31> 'void'
> | | | `-BlockExpr 0x5592888adb30 <col:10, col:29> 'void (^)()'
> | | | `-BlockDecl 0x5592888ad728 <col:10, col:29> col:10
> | | | |-capture Var 0x5592888ad318 'a' 'A':'A'
> | | | `-CompoundStmt 0x5592888ad930 <col:11, col:29>
> | | `-CXXFunctionalCastExpr 0x5592888b1278 <col:34, col:36> 'P':'P'
> functional cast to P <NoOp>
> | | `-ConstantExpr 0x5592888b1108 <col:34, col:36> 'P':'P'
> | | |-value: Struct
> | | `-ExprWithCleanups 0x5592888b10e8 <col:34, col:36> 'P':'P'
> | | |-cleanup Block 0x5592888ad728
> | | `-CXXTemporaryObjectExpr 0x5592888b10b8 <col:34, col:36>
> 'P':'P' 'void ()'
> | `-CXXBoolLiteralExpr 0x5592888b12c0 <col:39> 'bool' false
> ```
> I didn't check if this particular error can lead to codegen bug, but even
> having a misplaced cleanup in the AST is already a red flag.
>
> I think in case of immediate invocations we could simply create
> `ExprWithCleanups` and not do anything else:
> - `CleanupVarDeclMarking` will be handled when containing evaluation context
> is popped from the stack.
> - `ExprCleanupObjects` may only contain blocks at this point (as it's C++ and
> we have assert for this). One can't use blocks inside a constant expression
> (Clang will produce an error) so in correct code any cleanups we see inside
> the current context **must handled by the containing evaluation context** and
> not this `ExprWithCleanups`.
>
I see. Thank you. I updated it to only create `ExprWithCleanups`.
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
