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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:7374
   auto *E = ExprWithCleanups::Create(
       Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups);
 
----------------
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`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153962