yronglin added inline comments.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914
+ // [P2718R0] Lifetime extension in range-based for loops.
+ //
+ // 6.7.7 [class.temporary] p5:
+ // There are four contexts in which temporaries are destroyed at a different
+ // point than the end of the full-expression.
+ //
+ // 6.7.7 [class.temporary] p6:
----------------
hubert.reinterpretcast wrote:
> yronglin wrote:
> > yronglin wrote:
> > > hubert.reinterpretcast wrote:
> > > > yronglin wrote:
> > > > > rsmith wrote:
> > > > > > This isn't the right way to model the behavior here -- the presence
> > > > > > or absence of an `ExprWithCleanups` is just a convenience to tell
> > > > > > consumers of the AST whether they should expect to see cleanups
> > > > > > later or not, and doesn't carry an implication of affecting the
> > > > > > actual temporary lifetimes and storage durations.
> > > > > >
> > > > > > The outcome that we should be aiming to reach is that all
> > > > > > `MaterializeTemporaryExpr`s created as part of processing the
> > > > > > for-range-initializer are marked as being lifetime-extended by the
> > > > > > for-range variable. Probably the simplest way to handle that would
> > > > > > be to track the current enclosing for-range-initializer variable in
> > > > > > the `ExpressionEvaluationContextRecord`, and whenever a
> > > > > > `MaterializeTemporaryExpr` is created, if there is a current
> > > > > > enclosing for-range-initializer, mark that
> > > > > > `MaterializeTemporaryExpr` as being lifetime-extended by it.
> > > > > Awesome! Thanks a lot for your advice, this is very helpful! I want
> > > > > to take a longer look at it.
> > > > As mentioned in D139586, `CXXDefaultArgExpr`s may need additional
> > > > handling. Similarly for `CXXDefaultInitExpr`s.
> > > Thanks for your tips! I have a question that what's the correct way to
> > > extent the lifetime of `CXXBindTemporaryExpr`? Can I just `materialize`
> > > the temporary? It may replaced by `MaterializeTemporaryExpr`, and then I
> > > can mark it as being lifetime-extended by the for-range variable.
> > Eg.
> > ```
> > void f() {
> > int v[] = {42, 17, 13};
> > Mutex m;
> > for (int x : static_cast<void>(LockGuard(m)), v) // lock released in C++
> > 2020
> > {
> > LockGuard guard(m); // OK in C++ 2020, now deadlocks
> > }
> > }
> > ```
> > ```
> > BinaryOperator 0x135036220 'int[3]' lvalue ','
> > |-CXXStaticCastExpr 0x1350361d0 'void' static_cast<void> <ToVoid>
> > | `-CXXFunctionalCastExpr 0x135036198 'LockGuard':'struct LockGuard'
> > functional cast to LockGuard <ConstructorConversion>
> > | `-CXXBindTemporaryExpr 0x135036178 'LockGuard':'struct LockGuard'
> > (CXXTemporary 0x135036178)
> > | `-CXXConstructExpr 0x135036140 'LockGuard':'struct LockGuard' 'void
> > (Mutex &)'
> > | `-DeclRefExpr 0x135035e18 'Mutex':'class Mutex' lvalue Var
> > 0x135035b40 'm' 'Mutex':'class Mutex'
> > `-DeclRefExpr 0x135036200 'int[3]' lvalue Var 0x135035928 'v' 'int[3]'
> > ```
> If `MaterializeTemporaryExpr` represents a "temporary materialization
> conversion", then the above should already have one just under the
> `static_cast` to `void` (since the cast operand would be a discarded-value
> expression).
>
> There may be unfortunate effects from materializing temporaries for
> discarded-value expressions though: Technically, temporaries are also created
> for objects having scalar type.
>
> Currently, `MaterializeTemporaryExpr` is documented as being tied to
> reference binding, but that is not correct: for example,
> `MaterializeTemporaryExpr` also appears when a member access is made on a
> temporary of class type.
http://eel.is/c++draft/class.temporary says:
```
[Note 3: Temporary objects are materialized:
.......
(2.6)
when a prvalue that has type other than cv void appears as a discarded-value
expression ([expr.context]).
— end note]
```
Seems we should materialized the discard-value expression in this case, WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153701/new/
https://reviews.llvm.org/D153701
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
