hubert.reinterpretcast 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:
----------------
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.
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
