[PATCH] D52440: Emit lifetime markers for temporary function parameter aggregates

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D52440#1298816, @itessier wrote:

> > This is problematic because we're not necessarily in a scope that usefully limits the duration of cleanups — we don't push full-expression scopes when emitting an arbitrary statement. Probably we should, but we don't.
>
>
>
> > If you'd like to take a look at solving that problem first, that would be great.
>
> Sure I'd like to take a look at this, but I'm not familiar with the code gen module. Can you point me to where I should start looking to understand and figure out where to add scopes?


It's not as well documented as I'd like.  The main bits of infrastructure to understand are `CodeGenFunction::RunCleanupsScope` and the basic mechanics of pushing/popping cleanups in `CGCleanups`.  Then the basic idea is that you want to find a way to push a scope around any sort of "top-level" expression emission (i.e. whenever we emit an expression that isn't part of an outer full-expression — which is a language concept you might need to look up).  Fortunately, that's largely taken care of already because we have an `ExprWithCleanups` node that we create in Sema whenever there are certain kinds of semantic cleanups in a full-expression, and all the top-level places that emit expressions are probably already looking for that node to treat it specially.  (As an example of that, look for the call to `enterFullExpression` in `CodeGenFunction::EmitScalarInit`.

So the general guidelines for the patch are:

1. Introduce a `EmitIgnoredFullExpression` function that checks for `ExprWithCleanups`, calls `enterFullExpression` if necessary, pushes a scope, and then calls `EmitIgnoredExpr`.

2. Change the expression case of `CodeGenFunction::EmitStmt` to call `EmitIgnoredFullExpression`.  1+2 should be a stable intermediate point where you can fix any test changes and submit a patch.

3. Change the general emission cases for `ExprWithCleanups` to unconditionally assert.  (This means the case in `EmitLValue`, the cases in `CGExprScalar`, `CGExprComplex`, and `CGExprAgg`, and maybe a few others.)  The idea here is that *nothing* should be relying for correctness on just encountering an `ExprWithCleanups` at an arbitrary position within the expression tree — they should all be pushing scopes anyway, and as part of that they all need to be checking for `ExprWithCleanups`, so they should never just find it alone.

4. Deal with any ramifications of (3).

5. Look at all the uses of `ExprWithCleanups` / `enterFullExpression` in CodeGen to make sure that they indeed unconditionally push a scope around expression-emission.

At that point, we should be reasonably confident that we're actually pushing scopes around every expression emission.


Repository:
  rC Clang

https://reviews.llvm.org/D52440