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:
> 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?
I think we should, but what is the codegen fallout? Would no-opt builds start
writing `42` into allocated memory for `static_cast<void>(42)`?
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
