[Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[yronglin]

Summary

Implement P2718R0 "Lifetime extension in range-based for loops" (https://wg21.link/P2718R0)

[yronglin]

Update c++ status

[yronglin]

Format and rebase

[yronglin]

Rebase

[yronglin]

Poke CI

[yronglin]

friendly ping~

[yronglin]

Rebase

[cor3ntin]

Sorry, I missed the ping on Discord.
Thanks for working on this

I don't feel qualified to review this, but I don't think there are nearly enough tests.
FYI there is a previous attempt at this here https://reviews.llvm.org/D139586 - with some test suggestions

[yronglin]

In D153701#4459036, @cor3ntin wrote:

Sorry, I missed the ping on Discord.
Thanks for working on this

I don't feel qualified to review this, but I don't think there are nearly enough tests.
FYI there is a previous attempt at this here https://reviews.llvm.org/D139586 - with some test suggestions

Thank you for take a look! I'm sorry I have not found D139586 already exists, should I close this ticket? D139586 seems more complete.

[yronglin]

In D153701#4459036, @cor3ntin wrote:

Sorry, I missed the ping on Discord.
Thanks for working on this

I don't feel qualified to review this, but I don't think there are nearly enough tests.
FYI there is a previous attempt at this here https://reviews.llvm.org/D139586 - with some test suggestions

If you are no bandwidth to working on D139586, I'd be happy to do take over.

[rsmith]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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 MaterializeTemporaryExprs 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.

[yronglin]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

Awesome! Thanks a lot for your advice, this is very helpful! I want to take a longer look at it.

[hubert.reinterpretcast]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

As mentioned in D139586, CXXDefaultArgExprs may need additional handling. Similarly for CXXDefaultInitExprs.

[yronglin]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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.

[yronglin]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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]'

[cor3ntin]

Marking that as needing revision while the author explores the direction outlined by Richard :)

[hubert.reinterpretcast]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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.

[yronglin]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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?

[hubert.reinterpretcast]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

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)?

[yronglin]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

Thanks for your confirm @hubert.reinterpretcast !

I have tried locally, the generated IR of void f() is:

define void @f()() {
entry:
  %v = alloca [3 x i32], align 4
  %m = alloca %class.Mutex, align 8
  %__range1 = alloca ptr, align 8
  %ref.tmp = alloca %struct.LockGuard, align 8
  %__begin1 = alloca ptr, align 8
  %__end1 = alloca ptr, align 8
  %x = alloca i32, align 4
  %guard = alloca %struct.LockGuard, align 8
  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %v, ptr align 4 @__const.f().v, i64 12, i1 false)
  %call = call noundef ptr @Mutex::Mutex()(ptr noundef nonnull align 8 dereferenceable(64) %m)
  %call1 = call noundef ptr @LockGuard::LockGuard(Mutex&)(ptr noundef nonnull align 8 dereferenceable(8) %ref.tmp, ptr noundef nonnull align 8 dereferenceable(64) %m)
  store ptr %v, ptr %__range1, align 8
  %0 = load ptr, ptr %__range1, align 8
  %arraydecay = getelementptr inbounds [3 x i32], ptr %0, i64 0, i64 0
  store ptr %arraydecay, ptr %__begin1, align 8
  %1 = load ptr, ptr %__range1, align 8
  %arraydecay2 = getelementptr inbounds [3 x i32], ptr %1, i64 0, i64 0
  %add.ptr = getelementptr inbounds i32, ptr %arraydecay2, i64 3
  store ptr %add.ptr, ptr %__end1, align 8
  br label %for.cond

for.cond:                                         ; preds = %for.inc, %entry
  %2 = load ptr, ptr %__begin1, align 8
  %3 = load ptr, ptr %__end1, align 8
  %cmp = icmp ne ptr %2, %3
  br i1 %cmp, label %for.body, label %for.cond.cleanup

for.cond.cleanup:                                 ; preds = %for.cond
  %call5 = call noundef ptr @LockGuard::~LockGuard()(ptr noundef nonnull align 8 dereferenceable(8) %ref.tmp) #6
  br label %for.end

for.body:                                         ; preds = %for.cond
  %4 = load ptr, ptr %__begin1, align 8
  %5 = load i32, ptr %4, align 4
  store i32 %5, ptr %x, align 4
  %call3 = call noundef ptr @LockGuard::LockGuard(Mutex&)(ptr noundef nonnull align 8 dereferenceable(8) %guard, ptr noundef nonnull align 8 dereferenceable(64) %m)
  %call4 = call noundef ptr @LockGuard::~LockGuard()(ptr noundef nonnull align 8 dereferenceable(8) %guard) #6
  br label %for.inc

for.inc:                                          ; preds = %for.body
  %6 = load ptr, ptr %__begin1, align 8
  %incdec.ptr = getelementptr inbounds i32, ptr %6, i32 1
  store ptr %incdec.ptr, ptr %__begin1, align 8
  br label %for.cond

for.end:                                          ; preds = %for.cond.cleanup
  %call6 = call noundef ptr @Mutex::~Mutex()(ptr noundef nonnull align 8 dereferenceable(64) %m) #6
  ret void
}

The AST of void f() is:

|-FunctionDecl 0x14311d408 <line:62:1, line:69:1> line:62:6 used f 'void ()'
| `-CompoundStmt 0x143808898 <col:10, line:69:1>
|   |-DeclStmt 0x14311d710 <line:63:3, col:25>
|   | `-VarDecl 0x14311d528 <col:3, col:24> col:7 used v 'int[3]' cinit
|   |   `-InitListExpr 0x14311d648 <col:13, col:24> 'int[3]'
|   |     |-IntegerLiteral 0x14311d590 <col:14> 'int' 42
|   |     |-IntegerLiteral 0x14311d5b0 <col:18> 'int' 17
|   |     `-IntegerLiteral 0x14311d5d0 <col:22> 'int' 13
|   |-DeclStmt 0x14311d9f0 <line:64:3, col:10>
|   | `-VarDecl 0x14311d740 <col:3, col:9> col:9 used m 'Mutex':'Mutex' callinit destroyed
|   |   `-CXXConstructExpr 0x14311d9b0 <col:9> 'Mutex':'Mutex' 'void ()'
|   `-CXXForRangeStmt 0x143808700 <line:65:3, line:68:3>
|     |-<<<NULL>>>
|     |-DeclStmt 0x14311e2c0 <line:65:16>
|     | `-VarDecl 0x14311dfa8 <col:16, col:49> col:16 implicit used __range1 'int (&)[3]' cinit
|     |   `-ExprWithCleanups 0x14311e238 <col:16, col:49> 'int[3]' lvalue
|     |     `-BinaryOperator 0x14311de38 <col:16, col:49> 'int[3]' lvalue ','
|     |       |-CXXStaticCastExpr 0x14311dde8 <col:16, col:46> 'void' static_cast<void> <ToVoid>
|     |       | `-MaterializeTemporaryExpr 0x14311ddd0 <col:34, col:45> 'LockGuard':'LockGuard' xvalue extended by Var 0x14311dfa8 '__range1' 'int (&)[3]'
|     |       |   `-CXXFunctionalCastExpr 0x14311dd98 <col:34, col:45> 'LockGuard':'LockGuard' functional cast to LockGuard <ConstructorConversion>
|     |       |     `-CXXBindTemporaryExpr 0x14311dd78 <col:34, col:45> 'LockGuard':'LockGuard' (CXXTemporary 0x14311dd78)
|     |       |       `-CXXConstructExpr 0x14311dd40 <col:34, col:45> 'LockGuard':'LockGuard' 'void (Mutex &)'
|     |       |         `-DeclRefExpr 0x14311da18 <col:44> 'Mutex':'Mutex' lvalue Var 0x14311d740 'm' 'Mutex':'Mutex'
|     |       `-DeclRefExpr 0x14311de18 <col:49> 'int[3]' lvalue Var 0x14311d528 'v' 'int[3]'
|     |-DeclStmt 0x143808588 <col:14>
|     | `-VarDecl 0x14311e360 <col:14> col:14 implicit used __begin1 'int *':'int *' cinit
|     |   `-ImplicitCastExpr 0x143808400 <col:14> 'int *' <ArrayToPointerDecay>
|     |     `-DeclRefExpr 0x14311e2d8 <col:14> 'int[3]' lvalue Var 0x14311dfa8 '__range1' 'int (&)[3]'
|     |-DeclStmt 0x1438085a0 <col:14>
|     | `-VarDecl 0x143808248 <col:14, col:16> col:14 implicit used __end1 'int *':'int *' cinit
|     |   `-BinaryOperator 0x143808468 <col:14, col:16> 'int *' '+'
|     |     |-ImplicitCastExpr 0x143808450 <col:14> 'int *' <ArrayToPointerDecay>
|     |     | `-DeclRefExpr 0x14311e2f8 <col:14> 'int[3]' lvalue Var 0x14311dfa8 '__range1' 'int (&)[3]'
|     |     `-IntegerLiteral 0x143808430 <col:16> 'long' 3
|     |-BinaryOperator 0x143808628 <col:14> 'bool' '!='
|     | |-ImplicitCastExpr 0x1438085f8 <col:14> 'int *':'int *' <LValueToRValue>
|     | | `-DeclRefExpr 0x1438085b8 <col:14> 'int *':'int *' lvalue Var 0x14311e360 '__begin1' 'int *':'int *'
|     | `-ImplicitCastExpr 0x143808610 <col:14> 'int *':'int *' <LValueToRValue>
|     |   `-DeclRefExpr 0x1438085d8 <col:14> 'int *':'int *' lvalue Var 0x143808248 '__end1' 'int *':'int *'
|     |-UnaryOperator 0x143808668 <col:14> 'int *':'int *' lvalue prefix '++'
|     | `-DeclRefExpr 0x143808648 <col:14> 'int *':'int *' lvalue Var 0x14311e360 '__begin1' 'int *':'int *'
|     |-DeclStmt 0x14311dee0 <col:8, col:50>
|     | `-VarDecl 0x14311de78 <col:8, col:14> col:12 x 'int' cinit
|     |   `-ImplicitCastExpr 0x1438086d0 <col:14> 'int' <LValueToRValue>
|     |     `-UnaryOperator 0x1438086b8 <col:14> 'int' lvalue prefix '*' cannot overflow
|     |       `-ImplicitCastExpr 0x1438086a0 <col:14> 'int *':'int *' <LValueToRValue>
|     |         `-DeclRefExpr 0x143808680 <col:14> 'int *':'int *' lvalue Var 0x14311e360 '__begin1' 'int *':'int *'
|     `-CompoundStmt 0x143808880 <line:66:3, line:68:3>
|       `-DeclStmt 0x143808868 <line:67:5, col:23>
|         `-VarDecl 0x143808778 <col:5, col:22> col:15 guard 'LockGuard':'LockGuard' callinit destroyed
|           `-CXXConstructExpr 0x143808820 <col:15, col:22> 'LockGuard':'LockGuard' 'void (Mutex &)'
|             `-DeclRefExpr 0x1438087e0 <col:21> 'Mutex':'Mutex' lvalue Var 0x14311d740 'm' 'Mutex':'Mutex'

Would no-opt builds start writing 42 into allocated memory for static_cast<void>(42)?

I have got this result with my locally build clang

define noundef i32 @main() {
entry:
  %retval = alloca i32, align 4
  %ref.tmp = alloca i32, align 4
  store i32 0, ptr %retval, align 4
  store i32 42, ptr %ref.tmp, align 4
  call void @f()()
  ret i32 0
}

[hubert.reinterpretcast]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

Would no-opt builds start writing 42 into allocated memory for static_cast<void>(42)?

I have got this result with my locally build clang

define noundef i32 @main() {
entry:
  %retval = alloca i32, align 4
  %ref.tmp = alloca i32, align 4
  store i32 0, ptr %retval, align 4
  store i32 42, ptr %ref.tmp, align 4
  call void @f()()
  ret i32 0
}

@rsmith, in terms of the AST, do you have advice on the approach we should take in generating MaterializeTemporaryExprs for discarded-value expressions? Do we go with a language-specification purest direction and generate these for all prvalue object-typed discarded-value expressions? Do we limit generating them to range-for initializers? Do we further specialize and limit to non-scalar types or, even more specifically, to cases involving constructors or destructors?

[rsmith]

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

The cleanest solution seems to be that we create all the language-specified temporary materialization conversions; that approach will minimize the chance of subtle bugs in other areas of language semantics, and give a more faithful AST to external AST consumers. I think I have two questions:

1. How hard is it to teach CodeGen to still not create storage for a discarded temporary? I think we know when we're emitting a discarded expression, so we should have a reasonable place to hook into and skip the materialization.
2. What is the memory impact of creating these additional materializations? Eg, for a handful of fairly normal translation units, how much more space does the AST take?

I think the first step should be to add the code to create the new MTEs. Then we can explore points (1) and (2) to decide if we do so only within for-range initializers or everywhere.

[yronglin]

Try implement

[yronglin]

Thanks for your comments and sorry for the very late reply, I have been investigating how to achieve it these days.

clang/lib/Sema/SemaExprCXX.cpp

8913–8926

Seems it was ignored before: https://github.com/llvm/llvm-project/blob/cb63fa00d1f723e3b4e2fb5e6645595eb0a6e84c/clang/lib/Sema/SemaExprCXX.cpp#L8219-L8228

How hard is it to teach CodeGen to still not create storage for a discarded temporary?

I try to implement as wording by the standard and add a new flags variable CanIgnore to identify that the MaterializeTemporaryExpr can be ignored, but it broken 300+ tests, and I will try to fix these.

What is the memory impact of creating these additional materializations?

Can I just dump the status of BumpPtrAllocator? or does clang has any fancy option to do this?

[yronglin]

This is a very early implement, please give me some time to add and fix tests.

[yronglin]

Sorry for the late reply. I tried to investigate the memory impact of creating these additional materializations by build the whole llvm-project and compare the number of MaterializedTemporaryExpr created during parsing.

Steps:

1. Add a public member uint64_t NumMetarilizedTemporaryExpr = 0; in ASTContext .
2. Increment the value of NumMetarilizedTemporaryExpr in Sema::CreateMaterializeTemporaryExpr.
3. Write the NumMetarilizedTemporaryExpr into a text file when ASTContext destruction, each translation unit will append a line to the file to record the value of NumMetarilizedTemporaryExpr .
4. Build the entire llvm-project separately using the compiler that creates addational materializations and the compiler that doesn't.
5. Sum the numbers produced by each translation unit.

The result is:

Item	Count
Addational Materialized Temporarys Total	50655585
Clang Trunk Total	18346347
Diff	32309238

The more detail result in https://yronglin.github.io

The gap between these two numbers is very large. So I'think we can create additional materializations only within for-range initializers. I'm not sure if I can find a way to only create materializes for temporaries that need to have an extended lifetime, WDYT?

[hubert.reinterpretcast]

In D153701#4563919, @yronglin wrote:

The gap between these two numbers is very large. So I'think we can create additional materializations only within for-range initializers. I'm not sure if I can find a way to only create materializes for temporaries that need to have an extended lifetime, WDYT?

@rsmith asked for something slightly different (but I am not sure how to collect that info): He wanted the AST size delta.

For what we have, I think the detailed results table would be more useful if we add some size metric for the files and the increase expressed as a percentage. Perhaps sorting by file size would help contextualize the significance of the percentage change.

[yronglin]

In D153701#4575056, @hubert.reinterpretcast wrote:

In D153701#4563919, @yronglin wrote:

The gap between these two numbers is very large. So I'think we can create additional materializations only within for-range initializers. I'm not sure if I can find a way to only create materializes for temporaries that need to have an extended lifetime, WDYT?

@rsmith asked for something slightly different (but I am not sure how to collect that info): He wanted the AST size delta.

For what we have, I think the detailed results table would be more useful if we add some size metric for the files and the increase expressed as a percentage. Perhaps sorting by file size would help contextualize the significance of the percentage change.

Thanks to Aaron's suggestion in Discard, the initial idea was to count the number of MaterializedTemporaryExpr created in the process of building llvm-project, and then calculate the precise memory increment data, without being affected by the operating system state. Maybe I can refine this table, add a column to show the memory increment percentage change of each file.

[yronglin]

Only create addational metarialized temporary in for-range-init.
FIXME: Need to handle function default argument, and add more test to make sure the generated LLVM IR is correct.

[yronglin]

Handle default argument and dependent context.

[yronglin]

FIXME: Need add test in clang/test/CXX/special/class.temporary/p6.cpp to check generated LLVM IR.

[yronglin]

Should we dump the real default argument AST but not only CXXDefaultArgExpr if CXXDefaultArgExpr has been rewritted?

[yronglin]

Add test to check generated LLVM IR, and fix crash.

[yronglin]

@cor3ntin I have reused EnsureImmediateInvocationInDefaultArgs to rewrite CXXDefaultArgExpr, does this is a correct approach?

[yronglin]

In D153701#4563919, @yronglin wrote:

Sorry for the late reply. I tried to investigate the memory impact of creating these additional materializations by build the whole llvm-project and compare the number of MaterializedTemporaryExpr created during parsing.

Steps:

1. Add a public member uint64_t NumMetarilizedTemporaryExpr = 0; in ASTContext .
2. Increment the value of NumMetarilizedTemporaryExpr in Sema::CreateMaterializeTemporaryExpr.
3. Write the NumMetarilizedTemporaryExpr into a text file when ASTContext destruction, each translation unit will append a line to the file to record the value of NumMetarilizedTemporaryExpr .
4. Build the entire llvm-project separately using the compiler that creates addational materializations and the compiler that doesn't.
5. Sum the numbers produced by each translation unit.

The result is:

Item	Count
Addational Materialized Temporarys Total	50655585
Clang Trunk Total	18346347
Diff	32309238

The more detail result in https://yronglin.github.io

The gap between these two numbers is very large. So I'think we can create additional materializations only within for-range initializers. I'm not sure if I can find a way to only create materializes for temporaries that need to have an extended lifetime, WDYT?

I have updated the table, and it was sorted by Count Inc col.

Num files: 7445

Item	Count	Mem
Addational Materialized Temporarys Total	50655585	1187240.2734 (KB)
Clang Trunk Total	18346347	429992.5078 (KB)
Diff	32309238	757247.7656 (KB)
Avg	4339.7230	4.2380 (KB/file)

[yronglin]

friendly ping~

[yronglin]

Fix ci failure, and introduce an variable in ExpressionEvaluationContextRecord to rewrite default argument.

[yronglin]

Fix ci

[yronglin]

Do not spass MaterializePRValueInDiscardStatement in PushExpressionEvaluationContext.

[cor3ntin]

I'm really not qualified to give you meaningful feedback on design here, but I spotted a few minor things that can be improved, I hope it helps!

clang/include/clang/Sema/Sema.h

1357

Can IsRewriteDefaultArgument and MaterializePRValueInDiscardStatement have different values?
Maybe MaterializePRValueInDiscardStatement is sufficient

1359–1361

1361

Either IsInLifetimeExtendingContext or IsInCXXForRangeInitializer seem like better names

1363–1372

9854

clang/lib/Parse/ParseDecl.cpp

2246–2249

We need to decide whether we want to backport thgis behavior to older language mode, i think that would make sense.
@hubert.reinterpretcast wdyt?

2256–2260

A discarded statement is the non-taken branch of an if constexpr, so using discarded expression (or discarded value expression) will lead to less confusion.

clang/lib/Sema/SemaExpr.cpp

18168 ↗

(On Diff #551871)

Whitespace only change

clang/lib/Sema/SemaExprCXX.cpp

8212–8236

Correct me if I'm wrong but the thing we want to avoid here is to avoid creating unnecessary AST nodes, right?

clang/lib/Sema/SemaInit.cpp

7658

[yronglin]

@cor3ntin Thanks for your review!

clang/include/clang/Sema/Sema.h

1357

Currently they have the same value, but they mean different things, so I use two separated variables.

1361

Thanks for your suggestion, IsInLifetimeExtendingContext looks good to me, I can use this var name later.

clang/lib/Sema/SemaExpr.cpp

6250 ↗

(On Diff #551871)

@cor3ntin Does EnsureImmediateInvocationInDefaultArgs need to be renamed to a more generic name?

18168 ↗

(On Diff #551871)

Thanks, I'll remove this.

clang/lib/Sema/SemaExprCXX.cpp

8212–8236

Yeah, ignore unnecessary AST nodes and runtime memory allocation. The redundant alloca/store instructions may be removed during the optimization phase.

Eg.

static_cast<void>(42);

define noundef i32 @main() {
entry:
  %retval = alloca i32, align 4
  %ref.tmp = alloca i32, align 4
  store i32 0, ptr %retval, align 4
  store i32 42, ptr %ref.tmp, align 4
  call void @f()()
  ret i32 0
}

[yronglin]

Address comments

[yronglin]

ping~ @hubert.reinterpretcast @rsmith

[yronglin]

friendly ping~

[yronglin]

ping~

[yronglin]

Rebase

[yronglin]

ping～

[rsmith]

The changes in SemaInit.cpp don't look correct to me. Trying to recurse through the initializer and find every nested temporary is highly error-prone; I can't tell you which expression nodes you forgot to recurse through, but I'm sure there are some.

I think the approach being taken here is not really maintainable. We don't want the initialization code to need to know how to recurse through an expression after the fact and find all the temporaries that might need to be lifetime-extended, and we don't need it to do that either. Instead, we should make the expression evaluation context track the current for-range variable, and in Sema::CreateMaterializeTemporaryExpr, we should create a temporary that is already set to be lifetime-extended by the loop variable.

clang/include/clang/Sema/Sema.h

1356–1357

Can you expand this comment to more fully describe what this flag governs? Which default argument? How would it be rewritten?

clang/lib/Parse/ParseDecl.cpp

2246–2249

Definitely not. This is a visible behavior change to a rule that was not in any way incorrect or broken, and for which there were no implementation concerns. The change was not voted into the standard as a DR (see https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/n4929.html). Applying this behavior in older standard versions would be a conformance bug.

[yronglin]

Update

[yronglin]

@rsmith Thanks a lot for your comments and sorry for the very late reply, I was on vacation some time ago.

In D153701#4643609, @rsmith wrote:

The changes in SemaInit.cpp don't look correct to me. Trying to recurse through the initializer and find every nested temporary is highly error-prone; I can't tell you which expression nodes you forgot to recurse through, but I'm sure there are some.

I think the approach being taken here is not really maintainable. We don't want the initialization code to need to know how to recurse through an expression after the fact and find all the temporaries that might need to be lifetime-extended, and we don't need it to do that either. Instead, we should make the expression evaluation context track the current for-range variable, and in Sema::CreateMaterializeTemporaryExpr, we should create a temporary that is already set to be lifetime-extended by the loop variable.

I agree, I have reverted the changes in SemaInit.cpp. I have ever tried to do lifetime-extend in Sema::CreateMaterializeTemporaryExpr, but I fall into some trouble, the MaterializeTemporaryExpr was created before the for-range VarDecl, so we don't have a VarDecl for MaterializeTemporaryExpr::setExtendingDecl in Sema::CreateMaterializeTemporaryExpr, I have a possible solution here, Eg. we allocate a memory block which has same size of VarDecl, and pass this pointer to MaterializeTemporaryExpr::setExtendingDecl as a placeholder VarDecl, when we build the for-range statement, we just construct a VarDecl in the memory block that we allocated before. But this approach doesn't look very good and not maintainable. So I use ForRangeInitTemporaryLifetimeExtensionVisitor to visit every temporaries in the initializer and extend the lifetime. And ForRangeInitTemporaryLifetimeExtensionVisitor was derived from RecursiveASTVisitor, maybe this approach be able to handle all temporaries. WDYT?

clang/include/clang/Sema/Sema.h

1356–1357

Yeah, this variable has been removed. By default, all CallExprs in Clang share the same CXXDefaultArgExpr from parameters, but in some contexts (such as lifetime extension), the lifetime of the temporaries in the current default parameters needs to be extended, so the CXXDefaultArgExpr needs to be copied and the lifetime of temporaries in the copy need to be extended.

[yronglin]

ping~

[yronglin]

ping～

[yronglin]

friendly ping~

[yronglin]

ping

[Endill]

@yronglin We are sorry it takes so much time to get feedback. Richard and Hubert have little bandwidth for reviews. Others, including me, don't feel qualified to provide good feedback.

[yronglin]

In D153701#4656920, @Endill wrote:

@yronglin We are sorry it takes so much time to get feedback. Richard and Hubert have little bandwidth for reviews. Others, including me, don't feel qualified to provide good feedback.

@Endill Thanks for your reply, let's wait for them have time. You are clang experts, please feel free to comments.

[sammccall]

I'm also not an authority here, but would like to help get this unstuck. It seems like how to tie the MaterializeTemporaryExprs to the lifetime-extending decl is the biggest outstanding question?

In D153701#4653967, @yronglin wrote:

In D153701#4643609, @rsmith wrote:

I think the approach being taken here is not really maintainable. We don't want the initialization code to need to know how to recurse through an expression after the fact and find all the temporaries that might need to be lifetime-extended, and we don't need it to do that either. Instead, we should make the expression evaluation context track the current for-range variable, and in Sema::CreateMaterializeTemporaryExpr, we should create a temporary that is already set to be lifetime-extended by the loop variable.

I agree, I have reverted the changes in SemaInit.cpp. I have ever tried to do lifetime-extend in Sema::CreateMaterializeTemporaryExpr, but I fall into some trouble, the MaterializeTemporaryExpr was created before the for-range VarDecl, so we don't have a VarDecl for MaterializeTemporaryExpr::setExtendingDecl in Sema::CreateMaterializeTemporaryExpr

I think the RecursiveASTVisitor solution is still fragile.
Looking at BuildForRangeVarDecl, it seems it doesn't need any inputs that would preclude building it earlier, before parsing the range expression.
I haven't looked at *exactly* how to restructure the interaction between Parser/Sema to enable this, maybe it's a little ugly, but it seems justified in order to be able to use ExpressionEvaluationContextRecord as intended.

clang/lib/Sema/SemaStmt.cpp

2751

I think this will walk into the body of lambdas and extend the lifetime of temporaries found there.

You could explicitly prevent this, but this looks like the same sort of fragility Richard was concerned with: Expression evaluation context seems to be the right way to control the propagation, and it'd be nice not to reinvent it.

[yronglin]

Apply comments

[yronglin]

In D153701#4657427, @sammccall wrote:

I'm also not an authority here, but would like to help get this unstuck. It seems like how to tie the MaterializeTemporaryExprs to the lifetime-extending decl is the biggest outstanding question?

In D153701#4653967, @yronglin wrote:

In D153701#4643609, @rsmith wrote:

I think the approach being taken here is not really maintainable. We don't want the initialization code to need to know how to recurse through an expression after the fact and find all the temporaries that might need to be lifetime-extended, and we don't need it to do that either. Instead, we should make the expression evaluation context track the current for-range variable, and in Sema::CreateMaterializeTemporaryExpr, we should create a temporary that is already set to be lifetime-extended by the loop variable.

I agree, I have reverted the changes in SemaInit.cpp. I have ever tried to do lifetime-extend in Sema::CreateMaterializeTemporaryExpr, but I fall into some trouble, the MaterializeTemporaryExpr was created before the for-range VarDecl, so we don't have a VarDecl for MaterializeTemporaryExpr::setExtendingDecl in Sema::CreateMaterializeTemporaryExpr

I think the RecursiveASTVisitor solution is still fragile.
Looking at BuildForRangeVarDecl, it seems it doesn't need any inputs that would preclude building it earlier, before parsing the range expression.
I haven't looked at *exactly* how to restructure the interaction between Parser/Sema to enable this, maybe it's a little ugly, but it seems justified in order to be able to use ExpressionEvaluationContextRecord as intended.

Many thanks for your review!
I have canceled RecursiveASTVisitor plan. I try to use ExpressionEvaluationContextRecord and collect temporaries in CreateMaterializeTemporaryExpr when parsing for-range-init, and then extend lifetime of the collected temporaries in ActOnCXXForRangeStmt. WDYT?

[yronglin]

Fix ci

[yronglin]

Fix ci

[yronglin]

ping～

[yronglin]

friendly ping~

clang/include/clang/Sema/Sema.h

struct ExpressionEvaluationContextRecord {
     // A context can be nested in both a discarded statement context and
     // an immediate function context, so they need to be tracked independently.
     bool InDiscardedStatement;
     bool InImmediateFunctionContext;
     bool InImmediateEscalatingFunctionContext;
 
     bool IsCurrentlyCheckingDefaultArgumentOrInitializer = false;
 
+    bool ExtendCXXForRangeInitVariableLifetime = false;
+
+    bool MaterializePRValueTemporaryInDiscardStatement = false;
+
     // When evaluating immediate functions in the initializer of a default
     // argument or default member initializer, this is the declaration whose
     // default initializer is being evaluated and the location of the call
     // or constructor definition.
     struct InitializationContext {
       InitializationContext(SourceLocation Loc, ValueDecl *Decl,
                             DeclContext *Context)
           : Loc(Loc), Decl(Decl), Context(Context) {
         assert(Decl && Context && "invalid initialization context");
       }
 
       SourceLocation Loc;
       ValueDecl *Decl = nullptr;
       DeclContext *Context = nullptr;
     };
     std::optional<InitializationContext> DelayedDefaultInitializationContext;
 
     ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
                                       unsigned NumCleanupObjects,
                                       CleanupInfo ParentCleanup,
                                       Decl *ManglingContextDecl,

   }
 
   bool isImmediateFunctionContext() const {
     assert(!ExprEvalContexts.empty() &&
            "Must be in an expression evaluation context");
     return ExprEvalContexts.back().isImmediateFunctionContext();
   }
 
+  bool isExtendXXForRangeInitVariableLifetimeContext() const {
+    assert(!ExprEvalContexts.empty() &&
+           "Must be in an expression evaluation context");
+    return ExprEvalContexts.back().ExtendCXXForRangeInitVariableLifetime;
+  }
+
+  bool isMaterializePRValueTemporaryInDiscardStatement() const {
+    assert(!ExprEvalContexts.empty() &&
+           "Must be in an expression evaluation context");
+    return ExprEvalContexts.back().MaterializePRValueTemporaryInDiscardStatement;
+  }
+
   bool isCheckingDefaultArgumentOrInitializer() const {
     assert(!ExprEvalContexts.empty() &&
            "Must be in an expression evaluation context");
     const ExpressionEvaluationContextRecord &Ctx = ExprEvalContexts.back();
     return (Ctx.Context ==
             ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed) ||
            Ctx.IsCurrentlyCheckingDefaultArgumentOrInitializer;
   }
 

clang/lib/Parse/ParseDecl.cpp

Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
   // analyzed.
   //
   // Handle the Objective-C for-in loop variable similarly, although we
   // don't need to parse the container in advance.
   if (FRI && (Tok.is(tok::colon) || isTokIdentifier_in())) {
     bool IsForRangeLoop = false;
     if (TryConsumeToken(tok::colon, FRI->ColonLoc)) {
       IsForRangeLoop = true;
+      EnterExpressionEvaluationContext ForRangeInitContext(
+          Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+          /*LambdaContextDecl=*/nullptr,
+          Sema::ExpressionEvaluationContextRecord::EK_Other,
+          getLangOpts().CPlusPlus23);
+      if (getLangOpts().CPlusPlus23) {
+        auto &LastRecord = Actions.ExprEvalContexts.back();
+        LastRecord.MaterializePRValueTemporaryInDiscardStatement = true;
+      }
+
       if (getLangOpts().OpenMP)
         Actions.startOpenMPCXXRangeFor();
       if (Tok.is(tok::l_brace))
         FRI->RangeExpr = ParseBraceInitializer();
       else
         FRI->RangeExpr = ParseExpression();
     }
 
     Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);
     if (IsForRangeLoop) {
       Actions.ActOnCXXForRangeDecl(ThisDecl);
     } else {
       // Obj-C for loop
       if (auto *VD = dyn_cast_or_null<VarDecl>(ThisDecl))
         VD->setObjCForDecl(true);
     }
     Actions.FinalizeDeclaration(ThisDecl);

clang/lib/Sema/SemaExprCXX.cpp

ExprResult Sema::IgnoredValueConversions(Expr *E) {
   MaybeDecrementCount(E, RefsMinusAssignments);
 
   if (E->hasPlaceholderType()) {
     ExprResult result = CheckPlaceholderExpr(E);
     if (result.isInvalid()) return E;
     E = result.get();
   }
 
-  // C99 6.3.2.1:
-  //   [Except in specific positions,] an lvalue that does not have
-  //   array type is converted to the value stored in the
-  //   designated object (and is no longer an lvalue).
-  if (E->isPRValue()) {
-    // In C, function designators (i.e. expressions of function type)
-    // are r-values, but we still want to do function-to-pointer decay
-    // on them.  This is both technically correct and convenient for
-    // some clients.
-    if (!getLangOpts().CPlusPlus && E->getType()->isFunctionType())
-      return DefaultFunctionArrayConversion(E);
-
-    return E;
-  }
-
   if (getLangOpts().CPlusPlus) {
     // The C++11 standard defines the notion of a discarded-value expression;
     // normally, we don't need to do anything to handle it, but if it is a
     // volatile lvalue with a special form, we perform an lvalue-to-rvalue
     // conversion.
     if (getLangOpts().CPlusPlus11 && E->isReadIfDiscardedInCPlusPlus11()) {
       ExprResult Res = DefaultLvalueConversion(E);
       if (Res.isInvalid())
         return E;
       E = Res.get();
     } else {
       // Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if
       // it occurs as a discarded-value expression.
       CheckUnusedVolatileAssignment(E);
     }
 
     // C++1z:
     //   If the expression is a prvalue after this optional conversion, the
     //   temporary materialization conversion is applied.
     //
     // We skip this step: IR generation is able to synthesize the storage for
     // itself in the aggregate case, and adding the extra node to the AST is
     // just clutter.
     // FIXME: We don't emit lifetime markers for the temporaries due to this.
     // FIXME: Do any other AST consumers care about this?
+    if (isMaterializePRValueTemporaryInDiscardStatement() &&
+        getLangOpts().CPlusPlus17 && E->isPRValue() &&
+        !E->getType()->isVoidType()) {
+      ExprResult Res = TemporaryMaterializationConversion(E);
+      if (Res.isInvalid())
+        return E;
+      E = Res.get();
+    }
+    return E;
+  }
+
+  // C99 6.3.2.1:
+  //   [Except in specific positions,] an lvalue that does not have
+  //   array type is converted to the value stored in the
+  //   designated object (and is no longer an lvalue).
+  if (E->isPRValue()) {
+    // In C, function designators (i.e. expressions of function type)
+    // are r-values, but we still want to do function-to-pointer decay
+    // on them.  This is both technically correct and convenient for
+    // some clients.
+    if (!getLangOpts().CPlusPlus && E->getType()->isFunctionType())
+      return DefaultFunctionArrayConversion(E);
+
     return E;
   }
 
   // GCC seems to also exclude expressions of incomplete enum type.
   if (const EnumType *T = E->getType()->getAs<EnumType>()) {
     if (!T->getDecl()->isComplete()) {
       // FIXME: stupid workaround for a codegen bug!
       E = ImpCastExprToType(E, Context.VoidTy, CK_ToVoid).get();
       return E;
ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
   DeclContext *DC = CurContext;
   while (DC && isa<CapturedDecl>(DC))
     DC = DC->getParent();
   const bool IsInLambdaDeclContext = isLambdaCallOperator(DC);
   if (IsInLambdaDeclContext && CurrentLSI &&
       CurrentLSI->hasPotentialCaptures() && !FullExpr.isInvalid())
     CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(FE, CurrentLSI,
                                                               *this);
   return MaybeCreateExprWithCleanups(FullExpr);
 }
 
 StmtResult Sema::ActOnFinishFullStmt(Stmt *FullStmt) {
   if (!FullStmt) return StmtError();
 
   return MaybeCreateStmtWithCleanups(FullStmt);
 }
 
 Sema::IfExistsResult
 Sema::CheckMicrosoftIfExistsSymbol(Scope *S,
                                    CXXScopeSpec &SS,
                                    const DeclarationNameInfo &TargetNameInfo) {
   DeclarationName TargetName = TargetNameInfo.getName();
   if (!TargetName)
     return IER_DoesNotExist;
 
   // If the name itself is dependent, then the result is dependent.
   if (TargetName.isDependentName())
     return IER_Dependent;
 
   // Do the redeclaration lookup in the current scope.

clang/lib/Sema/SemaInit.cpp

 
 static bool pathContainsInit(IndirectLocalPath &Path) {
   return llvm::any_of(Path, [=](IndirectLocalPathEntry E) {
     return E.Kind == IndirectLocalPathEntry::DefaultInit ||
            E.Kind == IndirectLocalPathEntry::VarInit;
   });
 }
 
-static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
+static void visitLocalsRetainedByInitializer(Sema &S, IndirectLocalPath &Path,
                                              Expr *Init, LocalVisitor Visit,
                                              bool RevisitSubinits,
                                              bool EnableLifetimeWarnings);
 
-static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
+static void visitLocalsRetainedByReferenceBinding(Sema &S,
+                                                  IndirectLocalPath &Path,
                                                   Expr *Init, ReferenceKind RK,
                                                   LocalVisitor Visit,
                                                   bool EnableLifetimeWarnings);
 
 template <typename T> static bool isRecordWithAttr(QualType Type) {
   if (auto *RD = Type->getAsCXXRecordDecl())
     return RD->hasAttr<T>();
   return false;
static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
   } else if (FD->getReturnType()->isReferenceType()) {
     return llvm::StringSwitch<bool>(FD->getName())
         .Cases("get", "any_cast", true)
         .Default(false);
   }
   return false;
 }
 
-static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
+static void handleGslAnnotatedTypes(Sema &S, IndirectLocalPath &Path, Expr *Call,
                                     LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
     // We are not interested in the temporary base objects of gsl Pointers:
     //   Temp().ptr; // Here ptr might not dangle.
     if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
       return;
     // Once we initialized a value with a reference, it can no longer dangle.
     if (!Value) {
       for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
         if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
           continue;
         if (PE.Kind == IndirectLocalPathEntry::GslPointerInit)
           return;
         break;
       }
     }
     Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
                           : IndirectLocalPathEntry::GslReferenceInit,
                     Arg, D});
     if (Arg->isGLValue())
-      visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
+      visitLocalsRetainedByReferenceBinding(S, Path, Arg, RK_ReferenceBinding,
                                             Visit,
                                             /*EnableLifetimeWarnings=*/true);
     else
-      visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, Arg, Visit, true,
                                        /*EnableLifetimeWarnings=*/true);
     Path.pop_back();
   };
 
   if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
     const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee());
     if (MD && shouldTrackImplicitObjectArg(MD))
       VisitPointerArg(MD, MCE->getImplicitObjectArgument(),
if (RetT->isLValueReferenceType()) {
       if (Ctx.hasSameType(RetT, LHST))
         return true;
     }
   }
 
   return false;
 }
 
-static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
+static void visitLifetimeBoundArguments(Sema &S, IndirectLocalPath &Path, Expr *Call,
                                         LocalVisitor Visit) {
   const FunctionDecl *Callee;
   ArrayRef<Expr*> Args;
 
   if (auto *CE = dyn_cast<CallExpr>(Call)) {
     Callee = CE->getDirectCallee();
     Args = llvm::ArrayRef(CE->getArgs(), CE->getNumArgs());
   } else {
if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) {
     Args = Args.slice(1);
   } else if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
     ObjectArg = MCE->getImplicitObjectArgument();
   }
 
   auto VisitLifetimeBoundArg = [&](const Decl *D, Expr *Arg) {
     Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D});
     if (Arg->isGLValue())
-      visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
+      visitLocalsRetainedByReferenceBinding(S, Path, Arg, RK_ReferenceBinding,
                                             Visit,
                                             /*EnableLifetimeWarnings=*/false);
     else
-      visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, Arg, Visit, true,
                                        /*EnableLifetimeWarnings=*/false);
     Path.pop_back();
   };
 
   if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
     VisitLifetimeBoundArg(Callee, ObjectArg);
 
   for (unsigned I = 0,
                 N = std::min<unsigned>(Callee->getNumParams(), Args.size());
        I != N; ++I) {
-    if (Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
+    if (Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>() ||
+        S.isExtendXXForRangeInitVariableLifetimeContext())
       VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
   }
 }
 
 /// Visit the locals that would be reachable through a reference bound to the
 /// glvalue expression \c Init.
-static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
+static void visitLocalsRetainedByReferenceBinding(Sema &S, IndirectLocalPath &Path,
                                                   Expr *Init, ReferenceKind RK,
                                                   LocalVisitor Visit,
                                                   bool EnableLifetimeWarnings) {
   RevertToOldSizeRAII RAII(Path);
 
   // Walk past any constructs which we can lifetime-extend across.
   Expr *Old;
   do {
     Old = Init;
 
     if (auto *FE = dyn_cast<FullExpr>(Init))
       Init = FE->getSubExpr();
 
     if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
       // If this is just redundant braces around an initializer, step over it.
       if (ILE->isTransparent())
         Init = ILE->getInit(0);
     }
 
     // Step over any subobject adjustments; we may have a materialized
     // temporary inside them.
-    Init = const_cast<Expr *>(Init->skipRValueSubobjectAdjustments());
+    SmallVector<const Expr *, 32> CommaLHS;
+    SmallVector<SubobjectAdjustment, 32> Adjustments;
+    Init = const_cast<Expr *>(
+        Init->skipRValueSubobjectAdjustments(CommaLHS, Adjustments));
 
+    llvm::for_each(CommaLHS, [&](const Expr *LHS) {
+      visitLocalsRetainedByReferenceBinding(S, Path, const_cast<Expr *>(LHS),
+                                            RK_ReferenceBinding, Visit,
+                                            EnableLifetimeWarnings);
+    });
     // Per current approach for DR1376, look through casts to reference type
     // when performing lifetime extension.
     if (CastExpr *CE = dyn_cast<CastExpr>(Init))
       if (CE->getSubExpr()->isGLValue())
         Init = CE->getSubExpr();
 
     // Per the current approach for DR1299, look through array element access
     // on array glvalues when performing lifetime extension.
     if (auto *ASE = dyn_cast<ArraySubscriptExpr>(Init)) {
       Init = ASE->getBase();
       auto *ICE = dyn_cast<ImplicitCastExpr>(Init);
       if (ICE && ICE->getCastKind() == CK_ArrayToPointerDecay)
         Init = ICE->getSubExpr();
       else
         // We can't lifetime extend through this but we might still find some
         // retained temporaries.
-        return visitLocalsRetainedByInitializer(Path, Init, Visit, true,
+        return visitLocalsRetainedByInitializer(S, Path, Init, Visit, true,
                                                 EnableLifetimeWarnings);
     }
 
     // Step into CXXDefaultInitExprs so we can diagnose cases where a
     // constructor inherits one as an implicit mem-initializer.
     if (auto *DIE = dyn_cast<CXXDefaultInitExpr>(Init)) {
       Path.push_back(
           {IndirectLocalPathEntry::DefaultInit, DIE, DIE->getField()});
       Init = DIE->getExpr();
     }
   } while (Init != Old);
 
   if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init)) {
     if (Visit(Path, Local(MTE), RK))
-      visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, MTE->getSubExpr(), Visit, true,
                                        EnableLifetimeWarnings);
   }
 
   if (isa<CallExpr>(Init)) {
     if (EnableLifetimeWarnings)
-      handleGslAnnotatedTypes(Path, Init, Visit);
-    return visitLifetimeBoundArguments(Path, Init, Visit);
+      handleGslAnnotatedTypes(S, Path, Init, Visit);
+    return visitLifetimeBoundArguments(S, Path, Init, Visit);
   }
 
   switch (Init->getStmtClass()) {
   case Stmt::DeclRefExprClass: {
     // If we find the name of a local non-reference parameter, we could have a
     // lifetime problem.
     auto *DRE = cast<DeclRefExpr>(Init);
     auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
     if (VD && VD->hasLocalStorage() &&
         !DRE->refersToEnclosingVariableOrCapture()) {
       if (!VD->getType()->isReferenceType()) {
         Visit(Path, Local(DRE), RK);
       } else if (isa<ParmVarDecl>(DRE->getDecl())) {
         // The lifetime of a reference parameter is unknown; assume it's OK
         // for now.
         break;
       } else if (VD->getInit() && !isVarOnPath(Path, VD)) {
         Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
-        visitLocalsRetainedByReferenceBinding(Path, VD->getInit(),
+        visitLocalsRetainedByReferenceBinding(S, Path, VD->getInit(),
                                               RK_ReferenceBinding, Visit,
                                               EnableLifetimeWarnings);
       }
     }
     break;
   }
 
   case Stmt::UnaryOperatorClass: {
     // The only unary operator that make sense to handle here
     // is Deref.  All others don't resolve to a "name."  This includes
     // handling all sorts of rvalues passed to a unary operator.
     const UnaryOperator *U = cast<UnaryOperator>(Init);
     if (U->getOpcode() == UO_Deref)
-      visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, U->getSubExpr(), Visit, true,
                                        EnableLifetimeWarnings);
     break;
   }
 
   case Stmt::OMPArraySectionExprClass: {
-    visitLocalsRetainedByInitializer(Path,
+    visitLocalsRetainedByInitializer(S, Path,
                                      cast<OMPArraySectionExpr>(Init)->getBase(),
                                      Visit, true, EnableLifetimeWarnings);
     break;
   }
 
   case Stmt::ConditionalOperatorClass:
   case Stmt::BinaryConditionalOperatorClass: {
     auto *C = cast<AbstractConditionalOperator>(Init);
     if (!C->getTrueExpr()->getType()->isVoidType())
-      visitLocalsRetainedByReferenceBinding(Path, C->getTrueExpr(), RK, Visit,
+      visitLocalsRetainedByReferenceBinding(S, Path, C->getTrueExpr(), RK, Visit,
                                             EnableLifetimeWarnings);
     if (!C->getFalseExpr()->getType()->isVoidType())
-      visitLocalsRetainedByReferenceBinding(Path, C->getFalseExpr(), RK, Visit,
+      visitLocalsRetainedByReferenceBinding(S, Path, C->getFalseExpr(), RK, Visit,
                                             EnableLifetimeWarnings);
     break;
   }
 
   // FIXME: Visit the left-hand side of an -> or ->*.
 
   default:
     break;
   }
 }
 
 /// Visit the locals that would be reachable through an object initialized by
 /// the prvalue expression \c Init.
-static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
+static void visitLocalsRetainedByInitializer(Sema &S, IndirectLocalPath &Path,
                                              Expr *Init, LocalVisitor Visit,
                                              bool RevisitSubinits,
                                              bool EnableLifetimeWarnings) {
   RevertToOldSizeRAII RAII(Path);
 
   Expr *Old;
   do {
     Old = Init;
do {
     // Step over value-preserving rvalue casts.
     if (auto *CE = dyn_cast<CastExpr>(Init)) {
       switch (CE->getCastKind()) {
       case CK_LValueToRValue:
         // If we can match the lvalue to a const object, we can look at its
         // initializer.
         Path.push_back({IndirectLocalPathEntry::LValToRVal, CE});
         return visitLocalsRetainedByReferenceBinding(
-            Path, Init, RK_ReferenceBinding,
+            S, Path, Init, RK_ReferenceBinding,
             [&](IndirectLocalPath &Path, Local L, ReferenceKind RK) -> bool {
           if (auto *DRE = dyn_cast<DeclRefExpr>(L)) {
             auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
             if (VD && VD->getType().isConstQualified() && VD->getInit() &&
                 !isVarOnPath(Path, VD)) {
               Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
-              visitLocalsRetainedByInitializer(Path, VD->getInit(), Visit, true,
+              visitLocalsRetainedByInitializer(S, Path, VD->getInit(), Visit, true,
                                                EnableLifetimeWarnings);
             }
           } else if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L)) {
             if (MTE->getType().isConstQualified())
-              visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit,
+              visitLocalsRetainedByInitializer(S, Path, MTE->getSubExpr(), Visit,
                                                true, EnableLifetimeWarnings);
           }
           return false;
         }, EnableLifetimeWarnings);
 
         // We assume that objects can be retained by pointers cast to integers,
         // but not if the integer is cast to floating-point type or to _Complex.
         // We assume that casts to 'bool' do not preserve enough information to
if (auto *CE = dyn_cast<CastExpr>(Init)) {
       case CK_AnyPointerToBlockPointerCast:
       case CK_AddressSpaceConversion:
         break;
 
       case CK_ArrayToPointerDecay:
         // Model array-to-pointer decay as taking the address of the array
         // lvalue.
         Path.push_back({IndirectLocalPathEntry::AddressOf, CE});
-        return visitLocalsRetainedByReferenceBinding(Path, CE->getSubExpr(),
+        return visitLocalsRetainedByReferenceBinding(S, Path, CE->getSubExpr(),
                                                      RK_ReferenceBinding, Visit,
                                                      EnableLifetimeWarnings);
 
       default:
         return;
       }
 
       Init = CE->getSubExpr();
     }
   } while (Old != Init);
 
   // C++17 [dcl.init.list]p6:
   //   initializing an initializer_list object from the array extends the
   //   lifetime of the array exactly like binding a reference to a temporary.
   if (auto *ILE = dyn_cast<CXXStdInitializerListExpr>(Init))
-    return visitLocalsRetainedByReferenceBinding(Path, ILE->getSubExpr(),
+    return visitLocalsRetainedByReferenceBinding(S, Path, ILE->getSubExpr(),
                                                  RK_StdInitializerList, Visit,
                                                  EnableLifetimeWarnings);
 
   if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
     // We already visited the elements of this initializer list while
     // performing the initialization. Don't visit them again unless we've
     // changed the lifetime of the initialized entity.
     if (!RevisitSubinits)
       return;
 
     if (ILE->isTransparent())
-      return visitLocalsRetainedByInitializer(Path, ILE->getInit(0), Visit,
+      return visitLocalsRetainedByInitializer(S, Path, ILE->getInit(0), Visit,
                                               RevisitSubinits,
                                               EnableLifetimeWarnings);
 
     if (ILE->getType()->isArrayType()) {
       for (unsigned I = 0, N = ILE->getNumInits(); I != N; ++I)
-        visitLocalsRetainedByInitializer(Path, ILE->getInit(I), Visit,
+        visitLocalsRetainedByInitializer(S, Path, ILE->getInit(I), Visit,
                                          RevisitSubinits,
                                          EnableLifetimeWarnings);
       return;
     }
 
     if (CXXRecordDecl *RD = ILE->getType()->getAsCXXRecordDecl()) {
       assert(RD->isAggregate() && "aggregate init on non-aggregate");
 
       // If we lifetime-extend a braced initializer which is initializing an
       // aggregate, and that aggregate contains reference members which are
       // bound to temporaries, those temporaries are also lifetime-extended.
       if (RD->isUnion() && ILE->getInitializedFieldInUnion() &&
           ILE->getInitializedFieldInUnion()->getType()->isReferenceType())
-        visitLocalsRetainedByReferenceBinding(Path, ILE->getInit(0),
+        visitLocalsRetainedByReferenceBinding(S, Path, ILE->getInit(0),
                                               RK_ReferenceBinding, Visit,
                                               EnableLifetimeWarnings);
       else {
         unsigned Index = 0;
         for (; Index < RD->getNumBases() && Index < ILE->getNumInits(); ++Index)
-          visitLocalsRetainedByInitializer(Path, ILE->getInit(Index), Visit,
+          visitLocalsRetainedByInitializer(S, Path, ILE->getInit(Index), Visit,
                                            RevisitSubinits,
                                            EnableLifetimeWarnings);
         for (const auto *I : RD->fields()) {
           if (Index >= ILE->getNumInits())
             break;
           if (I->isUnnamedBitfield())
             continue;
           Expr *SubInit = ILE->getInit(Index);
           if (I->getType()->isReferenceType())
-            visitLocalsRetainedByReferenceBinding(Path, SubInit,
+            visitLocalsRetainedByReferenceBinding(S, Path, SubInit,
                                                   RK_ReferenceBinding, Visit,
                                                   EnableLifetimeWarnings);
           else
             // This might be either aggregate-initialization of a member or
             // initialization of a std::initializer_list object. Regardless,
             // we should recursively lifetime-extend that initializer.
-            visitLocalsRetainedByInitializer(Path, SubInit, Visit,
+            visitLocalsRetainedByInitializer(S, Path, SubInit, Visit,
                                              RevisitSubinits,
                                              EnableLifetimeWarnings);
           ++Index;
         }
       }
     }
     return;
   }
 
   // The lifetime of an init-capture is that of the closure object constructed
   // by a lambda-expression.
   if (auto *LE = dyn_cast<LambdaExpr>(Init)) {
     LambdaExpr::capture_iterator CapI = LE->capture_begin();
     for (Expr *E : LE->capture_inits()) {
       assert(CapI != LE->capture_end());
       const LambdaCapture &Cap = *CapI++;
       if (!E)
         continue;
       if (Cap.capturesVariable())
         Path.push_back({IndirectLocalPathEntry::LambdaCaptureInit, E, &Cap});
       if (E->isGLValue())
-        visitLocalsRetainedByReferenceBinding(Path, E, RK_ReferenceBinding,
+        visitLocalsRetainedByReferenceBinding(S, Path, E, RK_ReferenceBinding,
                                               Visit, EnableLifetimeWarnings);
       else
-        visitLocalsRetainedByInitializer(Path, E, Visit, true,
+        visitLocalsRetainedByInitializer(S, Path, E, Visit, true,
                                          EnableLifetimeWarnings);
       if (Cap.capturesVariable())
         Path.pop_back();
     }
   }
 
   // Assume that a copy or move from a temporary references the same objects
   // that the temporary does.
   if (auto *CCE = dyn_cast<CXXConstructExpr>(Init)) {
     if (CCE->getConstructor()->isCopyOrMoveConstructor()) {
       if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(CCE->getArg(0))) {
         Expr *Arg = MTE->getSubExpr();
         Path.push_back({IndirectLocalPathEntry::TemporaryCopy, Arg,
                         CCE->getConstructor()});
-        visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
+        visitLocalsRetainedByInitializer(S, Path, Arg, Visit, true,
                                          /*EnableLifetimeWarnings*/false);
         Path.pop_back();
       }
     }
   }
 
   if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) {
     if (EnableLifetimeWarnings)
-      handleGslAnnotatedTypes(Path, Init, Visit);
-    return visitLifetimeBoundArguments(Path, Init, Visit);
+      handleGslAnnotatedTypes(S, Path, Init, Visit);
+    return visitLifetimeBoundArguments(S, Path, Init, Visit);
   }
 
   switch (Init->getStmtClass()) {
   case Stmt::UnaryOperatorClass: {
     auto *UO = cast<UnaryOperator>(Init);
     // If the initializer is the address of a local, we could have a lifetime
     // problem.
     if (UO->getOpcode() == UO_AddrOf) {
       // If this is &rvalue, then it's ill-formed and we have already diagnosed
       // it. Don't produce a redundant warning about the lifetime of the
       // temporary.
       if (isa<MaterializeTemporaryExpr>(UO->getSubExpr()))
         return;
 
       Path.push_back({IndirectLocalPathEntry::AddressOf, UO});
-      visitLocalsRetainedByReferenceBinding(Path, UO->getSubExpr(),
+      visitLocalsRetainedByReferenceBinding(S, Path, UO->getSubExpr(),
                                             RK_ReferenceBinding, Visit,
                                             EnableLifetimeWarnings);
     }
     break;
   }
 
   case Stmt::BinaryOperatorClass: {
     // Handle pointer arithmetic.
     auto *BO = cast<BinaryOperator>(Init);
     BinaryOperatorKind BOK = BO->getOpcode();
     if (!BO->getType()->isPointerType() || (BOK != BO_Add && BOK != BO_Sub))
       break;
 
     if (BO->getLHS()->getType()->isPointerType())
-      visitLocalsRetainedByInitializer(Path, BO->getLHS(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, BO->getLHS(), Visit, true,
                                        EnableLifetimeWarnings);
     else if (BO->getRHS()->getType()->isPointerType())
-      visitLocalsRetainedByInitializer(Path, BO->getRHS(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, BO->getRHS(), Visit, true,
                                        EnableLifetimeWarnings);
     break;
   }
 
   case Stmt::ConditionalOperatorClass:
   case Stmt::BinaryConditionalOperatorClass: {
     auto *C = cast<AbstractConditionalOperator>(Init);
     // In C++, we can have a throw-expression operand, which has 'void' type
     // and isn't interesting from a lifetime perspective.
     if (!C->getTrueExpr()->getType()->isVoidType())
-      visitLocalsRetainedByInitializer(Path, C->getTrueExpr(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, C->getTrueExpr(), Visit, true,
                                        EnableLifetimeWarnings);
     if (!C->getFalseExpr()->getType()->isVoidType())
-      visitLocalsRetainedByInitializer(Path, C->getFalseExpr(), Visit, true,
+      visitLocalsRetainedByInitializer(S, Path, C->getFalseExpr(), Visit, true,
                                        EnableLifetimeWarnings);
     break;
   }
 
   case Stmt::BlockExprClass:
     if (cast<BlockExpr>(Init)->getBlockDecl()->hasCaptures()) {
       // This is a local block, whose lifetime is that of the function.
       Visit(Path, Local(cast<BlockExpr>(Init)), RK_ReferenceBinding);
void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
 
   auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
                               ReferenceKind RK) -> bool {
     SourceRange DiagRange = nextPathEntryRange(Path, 0, L);
     SourceLocation DiagLoc = DiagRange.getBegin();
 
     auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
 
+    if (MTE && isExtendXXForRangeInitVariableLifetimeContext()) {
+      MTE->setExtendingDecl(ExtendingEntity->getDecl(),
+                            ExtendingEntity->allocateManglingNumber());
+      return true;
+    }
+
     bool IsGslPtrInitWithGslTempOwner = false;
     bool IsLocalGslOwner = false;
     if (pathOnlyInitializesGslPointer(Path)) {
       if (isa<DeclRefExpr>(L)) {
         // We do not want to follow the references when returning a pointer originating
         // from a local owner to avoid the following false positive:
         //   int &p = *localUniquePtr;
         //   someContainer.add(std::move(localUniquePtr));
auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
     // warnings or errors on inner temporaries within this one's initializer.
     return false;
   };
 
   bool EnableLifetimeWarnings = !getDiagnostics().isIgnored(
       diag::warn_dangling_lifetime_pointer, SourceLocation());
   llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
   if (Init->isGLValue())
-    visitLocalsRetainedByReferenceBinding(Path, Init, RK_ReferenceBinding,
-                                          TemporaryVisitor,
+    visitLocalsRetainedByReferenceBinding(*this, Path, Init,
+                                          RK_ReferenceBinding, TemporaryVisitor,
                                           EnableLifetimeWarnings);
   else
-    visitLocalsRetainedByInitializer(Path, Init, TemporaryVisitor, false,
+    visitLocalsRetainedByInitializer(*this, Path, Init, TemporaryVisitor, false,
                                      EnableLifetimeWarnings);
 }
 
 static void DiagnoseNarrowingInInitList(Sema &S,
                                         const ImplicitConversionSequence &ICS,
                                         QualType PreNarrowingType,
                                         QualType EntityType,
                                         const Expr *PostInit);

clang/lib/Sema/SemaStmt.cpp

 #include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
 
   // Build  auto && __range = range-init
   // Divide by 2, since the variables are in the inner scope (loop body).
   const auto DepthStr = std::to_string(S->getDepth() / 2);
   SourceLocation RangeLoc = Range->getBeginLoc();
   VarDecl *RangeVar = BuildForRangeVarDecl(*this, RangeLoc,
                                            Context.getAutoRRefDeductType(),
                                            std::string("__range") + DepthStr);
+  {
+    EnterExpressionEvaluationContext RangeVarContext(
+        *this, ExpressionEvaluationContext::PotentiallyEvaluated, /*LambdaContextDecl=*/nullptr,
+        ExpressionEvaluationContextRecord::EK_Other, getLangOpts().CPlusPlus23);
+    if (getLangOpts().CPlusPlus23) {
+      auto &LastRecord = ExprEvalContexts.back();
+      LastRecord.ExtendCXXForRangeInitVariableLifetime = true;
+      Cleanup = LastRecord.ParentCleanup;
+    }
     if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
                               diag::err_for_range_deduction_failure)) {
       ActOnInitializerError(LoopVar);
       return StmtError();
     }
+  }
 
   // Claim the type doesn't contain auto: we've already done the checking.
   DeclGroupPtrTy RangeGroup =
       BuildDeclaratorGroup(MutableArrayRef<Decl *>((Decl **)&RangeVar, 1));
   StmtResult RangeDecl = ActOnDeclStmt(RangeGroup, RangeLoc, RangeLoc);
   if (RangeDecl.isInvalid()) {
     ActOnInitializerError(LoopVar);
     return StmtError();
static StmtResult RebuildForRangeWithDereference(Sema &SemaRef, Scope *S,
   SemaRef.Diag(RangeLoc, diag::err_for_range_dereference)
     << Range->getType() << FixItHint::CreateInsertion(RangeLoc, "*");
   return SemaRef.ActOnCXXForRangeStmt(
       S, ForLoc, CoawaitLoc, InitStmt, LoopVarDecl, ColonLoc,
       AdjustedRange.get(), RParenLoc, Sema::BFRK_Rebuild);
 }
 
 /// BuildCXXForRangeStmt - Build or instantiate a C++11 for-range statement.
 StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
                                       SourceLocation CoawaitLoc, Stmt *InitStmt,
                                       SourceLocation ColonLoc, Stmt *RangeDecl,
                                       Stmt *Begin, Stmt *End, Expr *Cond,
                                       Expr *Inc, Stmt *LoopVarDecl,
                                       SourceLocation RParenLoc,
                                       BuildForRangeKind Kind) {
   // FIXME: This should not be used during template instantiation. We should
   // pick up the set of unqualified lookup results for the != and + operators

clang/test/AST/ast-dump-for-range-lifetime.cpp

+// RUN: %clang_cc1 -std=c++23 -triple x86_64-linux-gnu -fcxx-exceptions -ast-dump %s \
+// RUN: | FileCheck -strict-whitespace %s
+
+namespace p2718r0 {
+struct T {
+  int a[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+  T() {}
+  ~T() {}
+  const int *begin() const { return a; }
+  const int *end() const { return a + 10; }
+};
+
+const T &f1(const T &t) { return t; }
+T g() { return T(); }
+
+void foo() {
+  // CHECK: FunctionDecl {{.*}} foo 'void ()'
+  // CHECK: `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:    |-<<<NULL>>>
+  // CHECK-NEXT:    |-DeclStmt {{.*}}
+  // CHECK-NEXT:    | `-VarDecl {{.*}} implicit used __range1 'const T &' cinit
+  // CHECK-NEXT:    |   `-ExprWithCleanups {{.*}} 'const T':'const p2718r0::T' lvalue
+  // CHECK-NEXT:    |     `-CallExpr {{.*}} 'const T':'const p2718r0::T' lvalue
+  // CHECK-NEXT:    |       |-ImplicitCastExpr {{.*}} 'const T &(*)(const T &)' <FunctionToPointerDecay>
+  // CHECK-NEXT:    |       | `-DeclRefExpr {{.*}} 'const T &(const T &)' lvalue Function {{.*}} 'f1' 'const T &(const T &)'
+  // CHECK-NEXT:    |       `-MaterializeTemporaryExpr {{.*}} 'const T':'const p2718r0::T' lvalue extended by Var {{.*}} '__range1' 'const T &'
+  // CHECK-NEXT:    |         `-ImplicitCastExpr {{.*}} 'const T':'const p2718r0::T' <NoOp>
+  // CHECK-NEXT:    |           `-CXXBindTemporaryExpr {{.*}} <col:20, col:22> 'T':'p2718r0::T' (CXXTemporary {{.*}})
+  // CHECK-NEXT:    |             `-CallExpr {{.*}} 'T':'p2718r0::T'
+  // CHECK-NEXT:    |               `-ImplicitCastExpr {{.*}} 'T (*)()' <FunctionToPointerDecay>
+  // CHECK-NEXT:    |                 `-DeclRefExpr {{.*}} 'T ()' lvalue Function {{.*}} 'g' 'T ()'
+  [[maybe_unused]] int sum = 0;
+  for (auto e : f1(g()))
+    sum += e;
+}
+
+struct LockGuard {
+    LockGuard(int) {}
+
+    ~LockGuard() {}
+};
+
+void f() {
+  int v[] = {42, 17, 13};
+  int M = 0;
+
+  // CHECK: FunctionDecl {{.*}} f 'void ()'
+  // CHECK: `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:   |-<<<NULL>>>
+  // CHECK-NEXT:   |-DeclStmt {{.*}}
+  // CHECK-NEXT:   | `-VarDecl {{.*}} col:16 implicit used __range1 'int (&)[3]' cinit
+  // CHECK-NEXT:   |   `-ExprWithCleanups {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT:   |     `-BinaryOperator {{.*}} 'int[3]' lvalue ','
+  // CHECK-NEXT:   |       |-CXXStaticCastExpr {{.*}}'void' static_cast<void> <ToVoid>
+  // CHECK-NEXT:   |       | `-MaterializeTemporaryExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' xvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:   |       |   `-CXXFunctionalCastExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' functional cast to LockGuard <ConstructorConversion>
+  // CHECK-NEXT:   |       |     `-CXXBindTemporaryExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' (CXXTemporary {{.*}})
+  // CHECK-NEXT:   |       |       `-CXXConstructExpr {{.*}} 'LockGuard':'p2718r0::LockGuard' 'void (int)'
+  // CHECK-NEXT:   |       |         `-ImplicitCastExpr {{.*}} 'int' <LValueToRValue>
+  // CHECK-NEXT:   |       |           `-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'M' 'int'
+  // CHECK-NEXT:   |       `-DeclRefExpr {{.*}} 'int[3]' lvalue Var {{.*}} 'v' 'int[3]'
+  for (int x : static_cast<void>(LockGuard(M)), v) // lock released in C++ 2020
+  {
+    LockGuard guard(M); // OK in C++ 2020, now deadlocks
+  }
+}
+
+} // namespace p2718r0