[Clang][C++23] Lifetime extension in range-based for loops

[cor3ntin]

Summary

Implemented the C++23 paper P2718R0

[rZhBoYao]

Consider:

struct T {
  const int *begin() const;
  const int *end()   const;
  T &r() [[clang::lifetimebound]];
  T t();
};

const T &f1(const T &t [[clang::lifetimebound]]) { return t; }
T g();

void foo() {
  for (auto e : f1(g())) {}
  for (auto e : g().t().t().r()) {}
}

The first __range1:

| | `-VarDecl 0x11e901358 <col:17, col:23> col:17 implicit used __range1 'const T &' cinit
| |   `-ExprWithCleanups 0x11e9015b8 <col:17, col:23> 'const T':'const T' lvalue
| |     `-CallExpr 0x11e9010b0 <col:17, col:23> 'const T':'const T' lvalue
| |       |-ImplicitCastExpr 0x11e901098 <col:17> 'const T &(*)(const T &)' <FunctionToPointerDecay>
| |       | `-DeclRefExpr 0x11e901018 <col:17> 'const T &(const T &)' lvalue Function 0x11e900a48 'f1' 'const T &(const T &)'
| |       `-MaterializeTemporaryExpr 0x11e9010f0 <col:20, col:22> 'const T':'const T' lvalue extended by Var 0x11e901358 '__range1' 'const T &'
| |         `-ImplicitCastExpr 0x11e9010d8 <col:20, col:22> 'const T':'const T' <NoOp>
| |           `-CallExpr 0x11e900ec0 <col:20, col:22> 'T':'T'
| |             `-ImplicitCastExpr 0x11e900ea8 <col:20> 'T (*)()' <FunctionToPointerDecay>
| |               `-DeclRefExpr 0x11e900e30 <col:20> 'T ()' lvalue Function 0x11e900bd0 'g' 'T ()'

g() is extended by __range1.

The second __range1:

| `-VarDecl 0x11e9040a0 <col:17, col:31> col:17 implicit used __range1 'T &' cinit
|   `-ExprWithCleanups 0x11e904338 <col:17, col:31> 'T':'T' lvalue
|     `-CXXMemberCallExpr 0x11e903f68 <col:17, col:31> 'T':'T' lvalue
|       `-MemberExpr 0x11e903f38 <col:17, col:29> '<bound member function type>' .r 0x11e900638
|         `-MaterializeTemporaryExpr 0x11e903f20 <col:17, col:27> 'T':'T' xvalue extended by Var 0x11e9040a0 '__range1' 'T &'
|           `-CXXMemberCallExpr 0x11e903f00 <col:17, col:27> 'T':'T'
|             `-MemberExpr 0x11e903ed0 <col:17, col:25> '<bound member function type>' .t 0x11e900780
|               `-MaterializeTemporaryExpr 0x11e903eb8 <col:17, col:23> 'T':'T' xvalue extended by Var 0x11e9040a0 '__range1' 'T &'
|                 `-CXXMemberCallExpr 0x11e903e68 <col:17, col:23> 'T':'T'
|                   `-MemberExpr 0x11e903e38 <col:17, col:21> '<bound member function type>' .t 0x11e900780
|                     `-MaterializeTemporaryExpr 0x11e903e20 <col:17, col:19> 'T':'T' xvalue extended by Var 0x11e9040a0 '__range1' 'T &'
|                       `-CallExpr 0x11e903e00 <col:17, col:19> 'T':'T'
|                         `-ImplicitCastExpr 0x11e903de8 <col:17> 'T (*)()' <FunctionToPointerDecay>
|                           `-DeclRefExpr 0x11e903dc8 <col:17> 'T ()' lvalue Function 0x11e900bd0 'g' 'T ()'

g() and 2 t()'s are extended by __range1.

[cor3ntin]

Thanks a lot for working on this.

I will try to do a more detailed review later, but at least I think we might want more tests. Maybe codegen tests that do not rely on [[clang::lifetimebound]], and tests with more chaining (a().b().c()) .

[royjacobson]

Thanks for picking this up! :)

The (non-wording) paper makes a pretty convincing case to just apply this retroactively to any C++11 code (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2644r1.pdf). I think we should apply this retroactively, maybe add a pedantic warning when we do a lifetime extension on code before C++23.

Also, +1 to more tests. Specifically constexpr and dependent contexts would be good to see.

[hubert.reinterpretcast]

In D139586#3987252, @royjacobson wrote:

The (non-wording) paper makes a pretty convincing case to just apply this retroactively to any C++11 code (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2644r1.pdf). I think we should apply this retroactively, maybe add a pedantic warning when we do a lifetime extension on code before C++23.

Just noting that the committee did not vote this in as a Defect Report, but I mostly agree that people should code for the new behaviour and that the old behaviour is unlikely to be relied on. I suspect this should be highlighted as technically a breaking change.

[hubert.reinterpretcast]

In D139586#3984118, @cor3ntin wrote:

I will try to do a more detailed review later, but at least I think we might want more tests. Maybe codegen tests that do not rely on [[clang::lifetimebound]], and tests with more chaining (a().b().c()) .

We need to cover AST structure more than chaining. For example the patch currently does not find the B() temporary:

struct A { ~A(); int x[3]; };
struct B : A {};
int (&f(const A *))[3];
const A *g(const A &);
void bar(int);
void foo() {
  for (auto e : f(g(B()))) {
    bar(e);
  }
}

[hubert.reinterpretcast]

At least on the surface, it looks like there will be a lot of trouble to deal with default arguments:

struct A { A(); ~A(); int x[3]; };
int (&f(const A & = A()))[3];
void bar(int);
void foo() {
  for (auto e : f()) { bar(e); }
}

We get a CXXDefaultArgExpr, and the expression on the parameter declaration is what has the MaterializeTemporaryExpr. It looks like we now have cases where different call sites require different semantics (and perhaps different diagnostics, somewhat like template instantiations).

[cor3ntin]

In D139586#4000309, @hubert.reinterpretcast wrote:

At least on the surface, it looks like there will be a lot of trouble to deal with default arguments:

struct A { A(); ~A(); int x[3]; };
int (&f(const A & = A()))[3];
void bar(int);
void foo() {
  for (auto e : f()) { bar(e); }
}

We get a CXXDefaultArgExpr, and the expression on the parameter declaration is what has the MaterializeTemporaryExpr. It looks like we now have cases where different call sites require different semantics (and perhaps different diagnostics, somewhat like template instantiations).

This PR https://reviews.llvm.org/D136554 adds some machinery to rewrite CXXDefaultArgExpr when they are used, so we could, if needed, reuse that machinery to inject MaterializeTemporaryExpr (as part of D136554 the rewrite only happen in the presence of immediate function calls)

[hubert.reinterpretcast]

Waiting on changes to fix the visitation to locate the appropriate temporaries.

[cor3ntin]

@rZhBoYao are you still working on this? Thanks!

[rZhBoYao]

In D139586#4324857, @cor3ntin wrote:

@rZhBoYao are you still working on this? Thanks!

No. Sorry that I don't have the capacity to work on this.

[cor3ntin]

In D139586#4342868, @rZhBoYao wrote:

In D139586#4324857, @cor3ntin wrote:

@rZhBoYao are you still working on this? Thanks!

No. Sorry that I don't have the capacity to work on this.

Thanks a lot for letting us know and getting this started. We will try to find someone to take over. Cheers!

[cor3ntin]

@hubert.reinterpretcast I'll try to look at that but unless I'm mistaken, the wording excludes function parameters

The fourth context is when a temporary object other than a function parameter object is created in the for-range-initializer of a range-based for statement. If such a temporary object would otherwise be destroyed at the end of the for-range-initializer full-expression, the object persists for the lifetime of the reference initialized by the for-range-initializer.

I think that invalidates some of your examples?

[cor3ntin]

FYI, I'm not actively working on this,
I did mess around with it a bit https://github.com/cor3ntin/llvm-project/commit/478ff7f1aa7a4046fa8b293dfb86489b930a8888
but I'm extremely unfamiliar with that part of clang and i have no clue how to write tests.
Feel free to take over!

[yronglin]

In D139586#4403533, @cor3ntin wrote:

FYI, I'm not actively working on this,
I did mess around with it a bit https://github.com/cor3ntin/llvm-project/commit/478ff7f1aa7a4046fa8b293dfb86489b930a8888
but I'm extremely unfamiliar with that part of clang and i have no clue how to write tests.
Feel free to take over!

In D139586#4403533, @cor3ntin wrote:

FYI, I'm not actively working on this,
I did mess around with it a bit https://github.com/cor3ntin/llvm-project/commit/478ff7f1aa7a4046fa8b293dfb86489b930a8888
but I'm extremely unfamiliar with that part of clang and i have no clue how to write tests.
Feel free to take over!

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

[cor3ntin]

The right way to do that is outline here https://reviews.llvm.org/D153701#4478240
There is no point in trying to keep this attempt around.

[hubert.reinterpretcast]

In D139586#4363725, @cor3ntin wrote:

@hubert.reinterpretcast I'll try to look at that but unless I'm mistaken, the wording excludes function parameters

The fourth context is when a temporary object other than a function parameter object is created in the for-range-initializer of a range-based for statement. If such a temporary object would otherwise be destroyed at the end of the for-range-initializer full-expression, the object persists for the lifetime of the reference initialized by the for-range-initializer.

I think that invalidates some of your examples?

@cor3ntin, the examples I gave were for temporary objects created for binding to a reference (that, in turn, happens to be a function parameter). Those temporary objects are not function parameters.

clang/docs/ReleaseNotes.rst

 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
 - Implemented `P1169R4: static operator() <https://wg21.link/P1169R4>`_ and `P2589R1: static operator[] <https://wg21.link/P2589R1>`_.
 - Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 <https://wg21.link/P2513R3>`_).
   This change was applied to C++20 as a Defect Report.
 - Implemented "Permitting static constexpr variables in constexpr functions" (`P2647R1 <https://wg21.link/P2647R1>_`).
+- Implemented "Lifetime extension in range-based for loops" (`P2718R0 <https://wg21.link/P2718R0>_`).
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------
 
  - Allow the use of ``__noinline__`` as a keyword (instead of ``__attribute__((noinline))``)
    in lambda declarations.
 
 Objective-C Language Changes in Clang

clang/include/clang/AST/Decl.h

class NonParmVarDeclBitfields {
     /// or an Objective-C @catch statement.
     unsigned ExceptionVar : 1;
 
     /// Whether this local variable could be allocated in the return
     /// slot of its function, enabling the named return value optimization
     /// (NRVO).
     unsigned NRVOVariable : 1;
 
+    /// Whether this variable is __range{depth} initialized by the
+    /// for-range-initializer in a C++0x for-range statement.
+    unsigned CXXRangeVar : 1;
+
     /// Whether this variable is the for-range-declaration in a C++0x
     /// for-range statement.
     unsigned CXXForRangeDecl : 1;
 
     /// Whether this variable is the for-in loop declaration in Objective-C.
     unsigned ObjCForDecl : 1;
 
     /// Whether this variable is (C++1z) inline.
public:
   bool isNRVOVariable() const {
     return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.NRVOVariable;
   }
   void setNRVOVariable(bool NRVO) {
     assert(!isa<ParmVarDecl>(this));
     NonParmVarDeclBits.NRVOVariable = NRVO;
   }
 
+  /// Determine whether this variable is __range{depth} initialized by the
+  /// for-range-initializer in a C++0x for-range statement.
+  bool isCXXRangeVar() const {
+    return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.CXXRangeVar;
+  }
+  void setCXXRangeVar(bool RV) {
+    assert(!isa<ParmVarDecl>(this));
+    NonParmVarDeclBits.CXXRangeVar = RV;
+  }
+
   /// Determine whether this variable is the for-range-declaration in
   /// a C++0x for-range statement.
   bool isCXXForRangeDecl() const {
     return isa<ParmVarDecl>(this) ? false : NonParmVarDeclBits.CXXForRangeDecl;
   }
   void setCXXForRangeDecl(bool FRD) {
     assert(!isa<ParmVarDecl>(this));
     NonParmVarDeclBits.CXXForRangeDecl = FRD;

clang/lib/Sema/SemaInit.cpp

 ///  * A BlockExpr for a block with captures.
 using Local = Expr*;
 
 /// Expressions we stepped over when looking for the local state. Any steps
 /// that would inhibit lifetime extension or take us out of subexpressions of
 /// the initializer are included.
 struct IndirectLocalPathEntry {
   enum EntryKind {
+    CXX2bForRangeInit,
     DefaultInit,
     AddressOf,
     VarInit,
     LValToRVal,
     LifetimeBoundCall,
     TemporaryCopy,
     LambdaCaptureInit,
     GslReferenceInit,
do {
 
     // 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();
     }
+
+    if (auto *CE = dyn_cast<CallExpr>(Init);
+        CE && !Path.empty() &&
+        Path.front().Kind == IndirectLocalPathEntry::CXX2bForRangeInit) {
+      if (EnableLifetimeWarnings)
+        handleGslAnnotatedTypes(Path, Init, Visit);
+      visitLifetimeBoundArguments(Path, Init, Visit);
+      if (auto *ME = dyn_cast<MemberExpr>(CE->getCallee())) {
+        Init = ME->getBase();
+      } else {
+        for (auto *RSE : CE->getRawSubExprs())
+          if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(RSE))
+            if (Visit(Path, Local(MTE), RK))
+              visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit,
+                                               true, EnableLifetimeWarnings);
+      }
+    }
   } while (Init != Old);
 
   if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init)) {
     if (Visit(Path, Local(MTE), RK))
       visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true,
                                        EnableLifetimeWarnings);
   }
 
if (auto *CE = dyn_cast<CastExpr>(Init)) {
                                                      EnableLifetimeWarnings);
 
       default:
         return;
       }
 
       Init = CE->getSubExpr();
     }
+
+    if (auto *CE = dyn_cast<CallExpr>(Init);
+        CE && !Path.empty() &&
+        Path.front().Kind == IndirectLocalPathEntry::CXX2bForRangeInit) {
+      if (EnableLifetimeWarnings)
+        handleGslAnnotatedTypes(Path, Init, Visit);
+      visitLifetimeBoundArguments(Path, Init, Visit);
+      if (auto *ME = dyn_cast<MemberExpr>(CE->getCallee())) {
+        visitLocalsRetainedByReferenceBinding(Path, ME->getBase(),
+                                              RK_ReferenceBinding, Visit,
+                                              EnableLifetimeWarnings);
+      } else {
+        for (auto *RSE : CE->getRawSubExprs())
+          if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(RSE))
+            visitLocalsRetainedByReferenceBinding(
+                Path, MTE, RK_ReferenceBinding, Visit, EnableLifetimeWarnings);
+      }
+    }
   } 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(),
                                                  RK_StdInitializerList, Visit,

 /// Determine whether this is an indirect path to a temporary that we are
 /// supposed to lifetime-extend along.
 static PathLifetimeKind
 shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
   PathLifetimeKind Kind = PathLifetimeKind::Extend;
   for (auto Elem : Path) {
     if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
       Kind = PathLifetimeKind::ShouldExtend;
-    else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
+    else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit &&
+             Path.front().Kind != IndirectLocalPathEntry::CXX2bForRangeInit)
       return PathLifetimeKind::NoExtend;
   }
   return Kind;
 }
 
 /// Find the range for the first interesting entry in the path at or after I.
 static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I,
                                       Expr *E) {
   for (unsigned N = Path.size(); I != N; ++I) {
     switch (Path[I].Kind) {
     case IndirectLocalPathEntry::AddressOf:
     case IndirectLocalPathEntry::LValToRVal:
     case IndirectLocalPathEntry::LifetimeBoundCall:
     case IndirectLocalPathEntry::TemporaryCopy:
     case IndirectLocalPathEntry::GslReferenceInit:
     case IndirectLocalPathEntry::GslPointerInit:
+    case IndirectLocalPathEntry::CXX2bForRangeInit:
       // These exist primarily to mark the path as not permitting or
       // supporting lifetime extension.
       break;
 
     case IndirectLocalPathEntry::VarInit:
       if (cast<VarDecl>(Path[I].D)->isImplicit())
         return SourceRange();
       [[fallthrough]];
auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
     }
 
     for (unsigned I = 0; I != Path.size(); ++I) {
       auto Elem = Path[I];
 
       switch (Elem.Kind) {
       case IndirectLocalPathEntry::AddressOf:
       case IndirectLocalPathEntry::LValToRVal:
+      case IndirectLocalPathEntry::CXX2bForRangeInit:
         // These exist primarily to mark the path as not permitting or
         // supporting lifetime extension.
         break;
 
       case IndirectLocalPathEntry::LifetimeBoundCall:
       case IndirectLocalPathEntry::TemporaryCopy:
       case IndirectLocalPathEntry::GslPointerInit:
       case IndirectLocalPathEntry::GslReferenceInit:
auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
     // We didn't lifetime-extend, so don't go any further; we don't need more
     // 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 (auto *VD = dyn_cast_or_null<VarDecl>(Entity.getDecl());
+      getLangOpts().CPlusPlus2b && VD && VD->isCXXRangeVar())
+    Path.push_back({IndirectLocalPathEntry::CXX2bForRangeInit, nullptr});
   if (Init->isGLValue())
     visitLocalsRetainedByReferenceBinding(Path, Init, RK_ReferenceBinding,
                                           TemporaryVisitor,
                                           EnableLifetimeWarnings);
   else
     visitLocalsRetainedByInitializer(Path, Init, TemporaryVisitor, false,
                                      EnableLifetimeWarnings);
 }

clang/lib/Sema/SemaStmt.cpp

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);
+  RangeVar->setCXXRangeVar(true);
   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 =

clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp

-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify=expected,pre-cxx2b %s
+// RUN: %clang_cc1 -std=c++14 -verify=expected,pre-cxx2b %s
+// RUN: %clang_cc1 -std=c++17 -verify=expected,pre-cxx2b %s
+// RUN: %clang_cc1 -std=c++2b -verify %s
 
 struct pr12960 {
   int begin;
   void foo(int x) {
     for (int& it : x) { // expected-error {{invalid range expression of type 'int'; no viable 'begin' function available}}
     }
   }
 };
void use(NA::A a, NB::B b, NC::C c, ND::D d, NE::E e, NF::F f) {
     for (auto x : a) {}
     for (auto x : b) {}
     for (auto x : c) {} // expected-error {{invalid range expression of type 'NC::C'; no viable 'end' function available}}
     for (auto x : d) {} // expected-error {{invalid range expression of type 'ND::D'; no viable 'begin' function available}}
     for (auto x : e) {} // expected-error {{invalid range expression of type 'NE::E'; no viable 'begin' function available}}
     for (auto x : f) {} // expected-error {{invalid range expression of type 'NF::F'; no viable 'end' function available}}
   }
 }
+
+namespace p2718r0 {
+struct T {
+  const int *begin() const;
+  const int *end()   const;
+  T &r() [[clang::lifetimebound]];
+};
+
+const T &f1(const T &t [[clang::lifetimebound]]) { return t; }
+T g();
+
+void foo() {
+  for (auto e : f1(g())) {} // pre-cxx2b-warning {{temporary implicitly bound to local reference will be destroyed at the end of the full-expression}}
+  for (auto e : g().r()) {} // pre-cxx2b-warning {{temporary implicitly bound to local reference will be destroyed at the end of the full-expression}}
+}
+}

clang/www/cxx_status.html

</tr>
     <tr>
       <td>consteval needs to propagate up (DR)</td>
       <td><a href="https://wg21.link/P2564R3">P2564R3</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>
       <td>Lifetime extension in range-based for loops</td>
       <td><a href="https://wg21.link/P2718R0">P2718R0</a></td>
-      <td class="none" align="center">No</td>
+      <td class="unreleased" align="center">Clang 16</td>
     </tr>
 </table>
 </details>
 
 <h2 id="dr">Defect reports</h2>
 
 <p>Clang generally aims to implement resolutions to Defect Reports (bug fixes
 against prior standards) retroactively, in all prior standard versions where