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

[rZhBoYao]

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.