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

[yronglin]

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

Differential Revision: https://reviews.llvm.org/D153701

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

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

Differential Revision: https://reviews.llvm.org/D153701

---

Patch is 43.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76361.diff

10 Files Affected:

• (modified) clang/include/clang/Parse/Parser.h (+1-1)
• (modified) clang/include/clang/Sema/Sema.h (+33-1)
• (modified) clang/lib/Parse/ParseDecl.cpp (+19)
• (modified) clang/lib/Parse/ParseStmt.cpp (+1-1)
• (modified) clang/lib/Sema/SemaExpr.cpp (+18-4)
• (modified) clang/lib/Sema/SemaExprCXX.cpp (+26-20)
• (modified) clang/lib/Sema/SemaInit.cpp (+4)
• (modified) clang/lib/Sema/SemaStmt.cpp (+9-1)
• (added) clang/test/AST/ast-dump-for-range-lifetime.cpp (+355)
• (modified) clang/test/CXX/special/class.temporary/p6.cpp (+234)

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 2dbe090bd0932f..a467ff6157962c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2377,7 +2377,7 @@ class Parser : public CodeCompletionHandler {
   struct ForRangeInit {
     SourceLocation ColonLoc;
     ExprResult RangeExpr;
-
+    SmallVector<MaterializeTemporaryExpr *, 8> LifetimeExtendTemps;
     bool ParsedForRangeDecl() { return !ColonLoc.isInvalid(); }
   };
   struct ForRangeInfo : ForRangeInit {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5e3b57ea33220b..250194f90d503c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1338,6 +1338,12 @@ class Sema final {
     /// context not already known to be immediately invoked.
     llvm::SmallPtrSet<DeclRefExpr *, 4> ReferenceToConsteval;
 
+    /// P2718R0 - Lifetime extension in range-based for loops.
+    /// MaterializeTemporaryExprs in for-range-init expression which need to
+    /// extend lifetime. Add MaterializeTemporaryExpr* if the value of
+    /// IsInLifetimeExtendingContext is true.
+    SmallVector<MaterializeTemporaryExpr *, 8> ForRangeLifetimeExtendTemps;
+
     /// \brief Describes whether we are in an expression constext which we have
     /// to handle differently.
     enum ExpressionKind {
@@ -1357,6 +1363,19 @@ class Sema final {
     // VLAs).
     bool InConditionallyConstantEvaluateContext = false;
 
+    /// Whether we are currently in a context in which temporaries must be
+    /// lifetime-extended (Eg. in a for-range initializer).
+    bool IsInLifetimeExtendingContext = false;
+
+    /// Whether we should materialize temporaries in discarded expressions.
+    ///
+    /// [C++23][class.temporary]/p2.6 when a prvalue that has type other than cv
+    /// void appears as a discarded-value expression ([expr.context]).
+    ///
+    /// We do not materialize temporaries by default in order to avoid creating
+    /// unnecessary temporary objects.
+    bool MaterializePRValueInDiscardedExpression = 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
@@ -5215,7 +5234,8 @@ class Sema final {
                                   Stmt *LoopVar,
                                   SourceLocation ColonLoc, Expr *Collection,
                                   SourceLocation RParenLoc,
-                                  BuildForRangeKind Kind);
+                                  BuildForRangeKind Kind,
+                                  ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps = {});
   StmtResult BuildCXXForRangeStmt(SourceLocation ForLoc,
                                   SourceLocation CoawaitLoc,
                                   Stmt *InitStmt,
@@ -9956,6 +9976,18 @@ class Sema final {
     return currentEvaluationContext().isImmediateFunctionContext();
   }
 
+  bool isInLifetimeExtendingContext() const {
+    assert(!ExprEvalContexts.empty() &&
+           "Must be in an expression evaluation context");
+    return ExprEvalContexts.back().IsInLifetimeExtendingContext;
+  }
+
+  bool ShouldMaterializePRValueInDiscardedExpression() const {
+    assert(!ExprEvalContexts.empty() &&
+           "Must be in an expression evaluation context");
+    return ExprEvalContexts.back().MaterializePRValueInDiscardedExpression;
+  }
+
   bool isCheckingDefaultArgumentOrInitializer() const {
     const ExpressionEvaluationContextRecord &Ctx = currentEvaluationContext();
     return (Ctx.Context ==
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index ed006f9d67de45..8b809aa9c3df26 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2312,12 +2312,31 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
     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);
+
+      // P2718R0 - Lifetime extension in range-based for loops.
+      if (getLangOpts().CPlusPlus23) {
+        auto &LastRecord = Actions.ExprEvalContexts.back();
+        LastRecord.IsInLifetimeExtendingContext = true;
+
+        // Materialize non-`cv void` prvalue temporaries in discarded
+        // expressions. These materialized temporaries may be lifetime-extented.
+        LastRecord.MaterializePRValueInDiscardedExpression = true;
+      }
+
       if (getLangOpts().OpenMP)
         Actions.startOpenMPCXXRangeFor();
       if (Tok.is(tok::l_brace))
         FRI->RangeExpr = ParseBraceInitializer();
       else
         FRI->RangeExpr = ParseExpression();
+      if (getLangOpts().CPlusPlus23)
+        FRI->LifetimeExtendTemps = std::move(
+            Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps);
     }
 
     Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index d0ff33bd1379ab..de883eb99765c9 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -2288,7 +2288,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
     ForRangeStmt = Actions.ActOnCXXForRangeStmt(
         getCurScope(), ForLoc, CoawaitLoc, FirstPart.get(),
         ForRangeInfo.LoopVar.get(), ForRangeInfo.ColonLoc, CorrectedRange.get(),
-        T.getCloseLocation(), Sema::BFRK_Build);
+        T.getCloseLocation(), Sema::BFRK_Build, ForRangeInfo.LifetimeExtendTemps);
 
   // Similarly, we need to do the semantic analysis for a for-range
   // statement immediately in order to close over temporaries correctly.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 960f513d1111b2..45f6f50e6f7385 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6256,7 +6256,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
   assert(Param->hasDefaultArg() && "can't build nonexistent default arg");
 
   bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
-
+  bool IsInLifetimeExtendingContext = isInLifetimeExtendingContext();
   std::optional<ExpressionEvaluationContextRecord::InitializationContext>
       InitializationContext =
           OutermostDeclarationWithDelayedImmediateInvocations();
@@ -6289,9 +6289,19 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
     ImmediateCallVisitor V(getASTContext());
     if (!NestedDefaultChecking)
       V.TraverseDecl(Param);
-    if (V.HasImmediateCalls) {
-      ExprEvalContexts.back().DelayedDefaultInitializationContext = {
-          CallLoc, Param, CurContext};
+
+    // Rewrite the call argument that was created from the corresponding
+    // parameter's default argument.
+    if (V.HasImmediateCalls || IsInLifetimeExtendingContext) {
+      if (V.HasImmediateCalls)
+        ExprEvalContexts.back().DelayedDefaultInitializationContext = {
+            CallLoc, Param, CurContext};
+      // Pass down lifetime extendning flag, and collect temporaries in
+      // CreateMaterializeTemporaryExpr when we rewrite the call argument.
+      auto &LastRecord = ExprEvalContexts.back();
+      auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
+      LastRecord.IsInLifetimeExtendingContext = IsInLifetimeExtendingContext;
+
       EnsureImmediateInvocationInDefaultArgs Immediate(*this);
       ExprResult Res;
       runWithSufficientStackSpace(CallLoc, [&] {
@@ -6305,6 +6315,10 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
       if (Res.isInvalid())
         return ExprError();
       Init = Res.get();
+
+      // Collect MaterializeTemporaryExprs in the rewrited CXXDefaultArgExpr.
+      PrevRecord.ForRangeLifetimeExtendTemps.append(
+          LastRecord.ForRangeLifetimeExtendTemps);
     }
   }
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4ae04358d5df7c..3590582fb5912e 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8209,21 +8209,6 @@ ExprResult Sema::IgnoredValueConversions(Expr *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
@@ -8244,11 +8229,32 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
     //   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?
+    // We do not materialize temporaries by default in order to avoid creating
+    // unnecessary temporary objects. If 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.
+    if (ShouldMaterializePRValueInDiscardedExpression() && 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;
   }
 
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 61d244f3bb9798..42e4d1eacce78d 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8469,6 +8469,10 @@ Sema::CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
   // are done in both CreateMaterializeTemporaryExpr and MaybeBindToTemporary,
   // but there may be a chance to merge them.
   Cleanup.setExprNeedsCleanups(false);
+  if (isInLifetimeExtendingContext()) {
+    auto &Record = ExprEvalContexts.back();
+    Record.ForRangeLifetimeExtendTemps.push_back(MTE);
+  }
   return MTE;
 }
 
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index f0b03db690843a..759453f319e2b5 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2487,7 +2487,8 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
                                       SourceLocation CoawaitLoc, Stmt *InitStmt,
                                       Stmt *First, SourceLocation ColonLoc,
                                       Expr *Range, SourceLocation RParenLoc,
-                                      BuildForRangeKind Kind) {
+                                      BuildForRangeKind Kind,
+                                      ArrayRef<MaterializeTemporaryExpr *> LifetimeExtendTemps) {
   // FIXME: recover in order to allow the body to be parsed.
   if (!First)
     return StmtError();
@@ -2539,6 +2540,12 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
     return StmtError();
   }
 
+  if (getLangOpts().CPlusPlus23) {
+    auto Entity = InitializedEntity::InitializeVariable(RangeVar);
+    for (auto *MTE : LifetimeExtendTemps)
+      MTE->setExtendingDecl(RangeVar, Entity.allocateManglingNumber());
+  }
+
   // Claim the type doesn't contain auto: we've already done the checking.
   DeclGroupPtrTy RangeGroup =
       BuildDeclaratorGroup(MutableArrayRef<Decl *>((Decl **)&RangeVar, 1));
@@ -2783,6 +2790,7 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
       LoopVar->setType(SubstAutoTypeDependent(LoopVar->getType()));
     }
   } else if (!BeginDeclStmt.get()) {
+
     SourceLocation RangeLoc = RangeVar->getLocation();
 
     const QualType RangeVarNonRefType = RangeVarType.getNonReferenceType();
diff --git a/clang/test/AST/ast-dump-for-range-lifetime.cpp b/clang/test/AST/ast-dump-for-range-lifetime.cpp
new file mode 100644
index 00000000000000..dec2e29184526e
--- /dev/null
+++ b/clang/test/AST/ast-dump-for-range-lifetime.cpp
@@ -0,0 +1,355 @@
+// RUN: %clang_cc1 -std=c++23 -triple x86_64-linux-gnu -fcxx-exceptions -ast-dump %s \
+// RUN: | FileCheck -strict-whitespace %s
+
+namespace P2718R0 {
+
+// Test basic
+struct A {
+  int a[3] = {1, 2, 3};
+  A() {}
+  ~A() {}
+  const int *begin() const { return a; }
+  const int *end() const { return a + 3; }
+  A& r() { return *this; }
+  A g() { return A(); }
+};
+
+A g() { return A(); }
+const A &f1(const A &t) { return t; }
+
+void test1() {
+  [[maybe_unused]] int sum = 0;
+  // CHECK: FunctionDecl {{.*}} test1 'void ()'
+  // CHECK:      |   `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT: |     |-<<<NULL>>>
+  // CHECK-NEXT: |     |-DeclStmt {{.*}}
+  // CHECK-NEXT: |     | `-VarDecl {{.*}} implicit used __range1 'const A &' cinit
+  // CHECK-NEXT: |     |   `-ExprWithCleanups {{.*}} 'const A':'const P2718R0::A' lvalue
+  // CHECK-NEXT: |     |     `-CallExpr {{.*}} 'const A':'const P2718R0::A' lvalue
+  // CHECK-NEXT: |     |       |-ImplicitCastExpr {{.*}} 'const A &(*)(const A &)' <FunctionToPointerDecay>
+  // CHECK-NEXT: |     |       | `-DeclRefExpr {{.*}} 'const A &(const A &)' lvalue Function {{.*}} 'f1' 'const A &(const A &)'
+  // CHECK-NEXT: |     |       `-MaterializeTemporaryExpr {{.*}} 'const A':'const P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'const A &'
+  // CHECK-NEXT: |     |         `-ImplicitCastExpr {{.*}} 'const A':'const P2718R0::A' <NoOp>
+  // CHECK-NEXT: |     |           `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
+  // CHECK-NEXT: |     |             `-CallExpr {{.*}} 'A':'P2718R0::A'
+  // CHECK-NEXT: |     |               `-ImplicitCastExpr {{.*}} 'A (*)()' <FunctionToPointerDecay>
+  // CHECK-NEXT: |     |                 `-DeclRefExpr {{.*}} 'A ()' lvalue Function {{.*}} 'g' 'A ()'
+  for (auto e : f1(g()))
+    sum += e;
+}
+
+struct B : A {};
+int (&f(const A *))[3];
+const A *g(const A &);
+void bar(int) {}
+
+void test2() {
+  // CHECK: FunctionDecl {{.*}} test2 'void ()'
+  // CHECK:      |   `-CXXForRangeStmt {{.*}}
+  // CHECK-NEXT: |     |-<<<NULL>>>
+  // CHECK-NEXT: |     |-DeclStmt {{.*}}
+  // CHECK-NEXT: |     | `-VarDecl {{.*}} implicit used __range1 'int (&)[3]' cinit
+  // CHECK-NEXT: |     |   `-ExprWithCleanups {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT: |     |     `-CallExpr {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT: |     |       |-ImplicitCastExpr {{.*}} 'int (&(*)(const A *))[3]' <FunctionToPointerDecay>
+  // CHECK-NEXT: |     |       | `-DeclRefExpr {{.*}} 'int (&(const A *))[3]' lvalue Function {{.*}} 'f' 'int (&(const A *))[3]'
+  // CHECK-NEXT: |     |       `-CallExpr {{.*}} 'const A *'
+  // CHECK-NEXT: |     |         |-ImplicitCastExpr {{.*}} 'const A *(*)(const A &)' <FunctionToPointerDecay>
+  // CHECK-NEXT: |     |         | `-DeclRefExpr {{.*}} 'const A *(const A &)' lvalue Function {{.*}} 'g' 'const A *(const A &)'
+  // CHECK-NEXT: |     |         `-ImplicitCastExpr {{.*}} 'const A':'const P2718R0::A' lvalue <DerivedToBase (A)>
+  // CHECK-NEXT: |     |           `-MaterializeTemporaryExpr {{.*}} 'const B':'const P2718R0::B' lvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT: |     |             `-ImplicitCastExpr {{.*}} 'const B':'const P2718R0::B' <NoOp>
+  // CHECK-NEXT: |     |               `-CXXBindTemporaryExpr {{.*}} 'B':'P2718R0::B' (CXXTemporary {{.*}})
+  // CHECK-NEXT: |     |                 `-CXXTemporaryObjectExpr {{.*}} 'B':'P2718R0::B' 'void () noexcept(false)' zeroing
+  for (auto e : f(g(B())))
+    bar(e);
+}
+
+// Test discard statement.
+struct LockGuard {
+    LockGuard() {}
+    ~LockGuard() {}
+};
+
+void test3() {
+  int v[] = {42, 17, 13};
+
+  // CHECK: FunctionDecl {{.*}} test3 'void ()'
+  // CHECK:      -CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:  |-<<<NULL>>>
+  // CHECK-NEXT:  |-DeclStmt {{.*}}
+  // CHECK-NEXT:  | `-VarDecl {{.*}} 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:  |       |   `-CXXBindTemporaryExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |       |     `-CXXTemporaryObjectExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' 'void ()'
+  // CHECK-NEXT:  |       `-DeclRefExpr {{.*}} 'int[3]' lvalue Var {{.*}} 'v' 'int[3]'
+  for ([[maybe_unused]] int x : static_cast<void>(LockGuard()), v)
+    LockGuard guard;
+
+  // CHECK:      -CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:  |-<<<NULL>>>
+  // CHECK-NEXT:  |-DeclStmt {{.*}}
+  // CHECK-NEXT:  | `-VarDecl {{.*}} implicit used __range1 'int (&)[3]' cinit
+  // CHECK-NEXT:  |   `-ExprWithCleanups {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT:  |     `-BinaryOperator {{.*}} 'int[3]' lvalue ','
+  // CHECK-NEXT:  |       |-CStyleCastExpr {{.*}} 'void' <ToVoid>
+  // CHECK-NEXT:  |       | `-MaterializeTemporaryExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' xvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |       |   `-CXXBindTemporaryExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |       |     `-CXXTemporaryObjectExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' 'void ()'
+  // CHECK-NEXT:  |       `-DeclRefExpr {{.*}} 'int[3]' lvalue Var {{.*}} 'v' 'int[3]'
+  for ([[maybe_unused]] int x : (void)LockGuard(), v)
+    LockGuard guard;
+
+  // CHECK:      -CXXForRangeStmt {{.*}}
+  // CHECK-NEXT:  |-<<<NULL>>>
+  // CHECK-NEXT:  |-DeclStmt {{.*}}
+  // CHECK-NEXT:  | `-VarDecl {{.*}} implicit used __range1 'int (&)[3]' cinit
+  // CHECK-NEXT:  |   `-ExprWithCleanups {{.*}} 'int[3]' lvalue
+  // CHECK-NEXT:  |     `-BinaryOperator {{.*}} 'int[3]' lvalue ','
+  // CHECK-NEXT:  |       |-MaterializeTemporaryExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' xvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |       | `-CXXBindTemporaryExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |       |   `-CXXTemporaryObjectExpr {{.*}} 'LockGuard':'P2718R0::LockGuard' 'void ()'
+  // CHECK-NEXT:  |       `-DeclRefExpr {{.*}} 'int[3]' lvalue Var {{.*}} 'v' 'int[3]'
+  for ([[maybe_unused]] int x : LockGuard(), v)
+    LockGuard guard;
+}
+
+// Test default arg
+int (&default_arg_fn(const A & = A()))[3];
+void test4() {
+
+  // CHECK: FunctionDecl {{.*}} test4 'void ()'
+  // FIXME: Should dump CXXDefaultArgExpr->getExpr() if CXXDefaultArgExpr has been rewrited?
+  for (auto e : default_arg_fn()) 
+    bar(e);
+}
+
+struct DefaultA {
+  DefaultA() {}
+  ~DefaultA() {}
+};
...
[truncated]

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[yronglin]

ping~

[AaronBallman]

Thank you for this! I think it looks pretty close, but I'd love to get more eyes on this given the importance of the feature. CC @zygoloid @rjmccall @erichkeane

[AaronBallman]

Can you explain why the check for C++17?

[yronglin]

AFAIK, https://en.cppreference.com/w/cpp/language/lifetime , since C++17, we should materialize temporary object when a prvalue appears as a discarded-value expression, and the materialization of a temporary object is generally delayed as long as possible in order to avoid creating unnecessary temporary object, perviously, we ignored it, since this patch, we materialize temporary object once there requires to extending lifetime.

[AaronBallman]

Ah okay, so this is still related to the lifetime extension for range-based for loops -- we needed this bit for C++17 conformance which we previously didn't have to do?

(Mostly trying to reason about whether this is a separate bug fix that should be landed in a separate patch or whether this is related to the correctness for P2718R0.)

[yronglin]

Yeah, I think so. we previously didn't materialize temporary object may should also be in compliance with the standard words "the materialization of a temporary object is generally delayed as long as possible in order to avoid creating unnecessary temporary object".

[yronglin]

Add ReleaseNotes and address Aaron's comments.

[yronglin]

Rebase to fix CI issue

[cor3ntin]

@zygoloid Can you take another crack at this? If you have time to confirm the design maybe we could try to land it in Clang 18. Otherwise the plan is to be conservative. Thanks!

[zygoloid]

This looks good to me.

[zygoloid]

Is there a reason that this is capitalized and the function above is not? (Genuine question, I've lost track of LLVM's preferred capitalization rule for functions, but this locally looks inconsistent.)

[yronglin]

LLVM prefer shouldMaterializePRValueInDiscardedExpression https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly , I'll rename it.

[zygoloid]

I think we should be clear here that this is only for unusual lifetime-extension contexts, and not just (for example) in an initializer of a reference.

Suggested change

	/// Whether we are currently in a context in which temporaries must be
	/// lifetime-extended (Eg. in a for-range initializer).
	bool IsInLifetimeExtendingContext = false;
	/// Whether we are currently in a context in which all temporaries must be
	/// lifetime-extended, even if they're not bound to a reference (for example,
	/// in a for-range initializer).
	bool IsInLifetimeExtendingContext = false;

[yronglin]

agree

[shafik]

Why not move this into the if (getLangOpts().CPlusPlus23) above?

[cor3ntin]

ForRangeLifetimeExtendTemps is going to be filled by ParseExpression/ I wonder if we should do that unconditionally though.
and replace the if with assert(getLangOpts().CPlusPlus23 || Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps.empty())

[yronglin]

agree

[shafik]

Is it possible to also test the case where it does not clean up like in the proposal:

for (auto e : f2(g())) {}  // undefined behavior

[yronglin]

agree, I have compiled this case, Currently, clang will not extending lifetime of temporaries in this case.

[cor3ntin]

Suggested change

	/// MaterializeTemporaryExprs in for-range-init expression which need to
	/// MaterializeTemporaryExprs in for-range-init expressions which need to

[cor3ntin]

ForRangeLifetimeExtendTemps is going to be filled by ParseExpression/ I wonder if we should do that unconditionally though.
and replace the if with assert(getLangOpts().CPlusPlus23 || Actions.ExprEvalContexts.back().ForRangeLifetimeExtendTemps.empty())

[cor3ntin]

We probably should do the same thing for BuildCXXDefaultInitExpr, right?

struct my_range {
  // ...
  foo bar = temp();
};

for(elem : my_range {}) {}

[yronglin]

Good catch! Yeah, we need do the same thing for BuildCXXDefaultInitExpr, I'm working on it.

[cor3ntin]

@yronglin Do you know when you will be able to get to this? thanks!

[yronglin]

@yronglin Do you know when you will be able to get to this? thanks!

Sorry for the lack of new actions for a long time. I have recently fixed some problems in the dependent context. I will try to complete the DefaultInit related functions and tests this weekend.

[yronglin]

I think this variable and related function shouldMaterializePRValueInDiscardedExpression() should be renamed to NeedMaterializePRValue and needMaterializePRValue() , then it can use in other cases.

In [class.temporary P2][http://eel.is/c++draft/class.temporary#2]

The materialization of a temporary object is generally delayed as long as possible in order to avoid creating unnecessary temporary objects.
[Note 3: Temporary objects are materialized:
(2.1)
when binding a reference to a prvalue ([dcl.init.ref], [expr.type.conv], [expr.dynamic.cast], [expr.static.cast], [expr.const.cast], [expr.cast]),
(2.2)
when performing member access on a class prvalue ([expr.ref], [expr.mptr.oper]),
(2.3)
when performing an array-to-pointer conversion or subscripting on an array prvalue ([conv.array], [expr.sub]),
(2.4)
when initializing an object of type std​::​initializer_list from a braced-init-list ([dcl.init.list]),
(2.5)
for certain unevaluated operands ([expr.typeid], [expr.sizeof]), and
(2.6)
when a prvalue that has type other than cv void appears as a discarded-value expression ([expr.context]).
— end note]

[yronglin]

Should we also dump the sub-expr which in CXXDefaultArgExpr:
for example:
Consider:

struct A {
  A();
  ~A();
};

typedef int vec3[3];
vec3 &f(const A &a = A{});

void foo() {
  for (auto e : f()) {}
}

Current AST Dump:

FunctionDecl 0x126813170 <line:85:1, line:87:1> line:85:6 foo 'void ()'
  `-CompoundStmt 0x126814720 <col:12, line:87:1>
    `-CXXForRangeStmt 0x1268146b0 <line:86:3, col:23>
      |-<<<NULL>>>
      |-DeclStmt 0x1268137b8 <col:17>
      | `-VarDecl 0x126813548 <col:17, col:19> col:17 implicit used __range1 'vec3 &' cinit
      |   `-ExprWithCleanups 0x126813730 <col:17, col:19> 'vec3':'int[3]' lvalue
      |     `-CallExpr 0x126813318 <col:17, col:19> 'vec3':'int[3]' lvalue
      |       |-ImplicitCastExpr 0x126813300 <col:17> 'vec3 &(*)(const A &)' <FunctionToPointerDecay>
      |       | `-DeclRefExpr 0x126813278 <col:17> 'vec3 &(const A &)' lvalue Function 0x126813030 'f' 'vec3 &(const A &)'
      |       `-CXXDefaultArgExpr 0x1268133f0 <<invalid sloc>> 'const A' lvalue
      |-DeclStmt 0x126814448 <col:15>
      | `-VarDecl 0x126813880 <col:15> col:15 implicit used __begin1 'int *' cinit
 .......

We can't see directly on the AST whether the lifetime is extended or not. The sub-expr which in CXXDefaultArgExpr is:

ExprWithCleanups 0x1398c55d8 'const A':'const struct A' lvalue
`-MaterializeTemporaryExpr 0x1398c55c0 'const A':'const struct A' lvalue extended by Var 0x1398c5748 '__range1' 'vec3 &'
  `-ImplicitCastExpr 0x1398c55a8 'const A':'const struct A' <NoOp>
    `-CXXBindTemporaryExpr 0x1398c5588 'A':'struct A' (CXXTemporary 0x1398c5588)
      `-CXXTemporaryObjectExpr 0x1398c5550 'A':'struct A' 'void (void)'

[cor3ntin]

Should we also dump the sub-expr which in CXXDefaultArgExpr: for example: Consider:

I think this makes sense, but it can be a separate patch, unless it helps for testing.

FYI, we prefer merge commits instead of rebase + force push because Github reviews tend to loose context when doing force pushes / rebases in PRs

[cor3ntin]

Where are the tests for BuildCXXDefaultInitExpr?

[yronglin]

Thanks for your review! I still have some questions about member init.
For this case(https://godbolt.org/z/jPKe8M6xE), should we materialize the temporaries in class Z::x's default init, but unless I'm mistaken?

[yronglin]

I tried this code https://godbolt.org/z/n77v9rMTz , and the temporaries which in CXXDefaultInitExpr has been lifetime extended in AST. But the LLVM IR unexpected, and the lifetime of the temporaries was not extended. Could you please guide me as to why this is and whether I have made some mistake?

[cor3ntin]

I think part of the problem might be that we should only extend in the aggregate case.
ie, when we call BuildCXXDefaultInit from FillInEmptyInitForField.

In the constructor case, the init lifetime is bound to that of the member initializer expression.

[yronglin]

Thanks for your help! I have a case use aggregate initialization (https://godbolt.org/z/vn8Y11ja8). I think neither the brace-or-equals initializer nor default member initializer should not be materialized and lifetime-extended in this code snippet, seems they don't satisfy any of class.temporary P2 . @hubert-reinterpretcast Could you please give me an example where CXXDefaultInitExpr has to be lifetime-extended? I saw you comment in D153701 that we need to do lifetime-extend for CXXDefaultInitExpr. I'm sorry that I'm unfamiliar with that part of clang and I have no clue how to write tests for CXXDefaultInitExpr.

[cor3ntin]

I think you are right.
I'm sorry, that whole thing is a bit confusing to me.

But the initializer is used in the construction of a subobject, which is different from a default argument which is initialized on call site... so I think the conclusion is that we should do nothing for default inits, I think

[yronglin]

Thank you for your consideration. I think so too, whether I have made some mistake, I will try to ask the author of P2718R0

[hubert-reinterpretcast]

@hubert-reinterpretcast Could you please give me an example where CXXDefaultInitExpr has to be lifetime-extended?

#85613

[yronglin]

Many thanks for your information.

[yronglin]

For this case, should

Should we also dump the sub-expr which in CXXDefaultArgExpr: for example: Consider:

I think this makes sense, but it can be a separate patch, unless it helps for testing.

Cool! Let's do it in a separate patch.

FYI, we prefer merge commits instead of rebase + force push because Github reviews tend to loose context when doing force pushes / rebases in PRs

Thanks for the reminder, I may not have converted from the Phabricator habit yet. I tried to rebase to fix a libcxx ci issue(gdb_pretty_printer_test)

[cor3ntin]

Here is what I'd like to do @yronglin
Revert the changes to BuildCXXDefaultInit.
And land it in main.
Then we'll have a few weeks to decide if we backport it to 18. Dies that sound good to you?

[cor3ntin]

Suggested change

	<td class="none" align="center">Clang 18</td>
	<td class="none" align="center">Clang 19</td>

[yronglin]

done

[yronglin]

Here is what I'd like to do @yronglin Revert the changes to BuildCXXDefaultInit. And land it in main. Then we'll have a few weeks to decide if we backport it to 18. Dies that sound good to you?

Yeah, it's looks good to me, I have reverted changes for CXXDefaultInitExpr.

[cor3ntin]

LGTM.
Do you need me to merge that for you?

[yronglin]

LGTM. Do you need me to merge that for you?

Thanks a lot for your review and help! I have commit access, once the CI green, I'll land this patch.