[Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer

[yronglin]

This PR complete DR1815 under the guidance of FIXME comments. And reuse CXXDefaultInitExpr rewrite machinery to clone the initializer expression on each use that would lifetime extend its temporaries.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

This PR complete DR1815 under the guidance of FIXME comments. And reuse CXXDefaultInitExpr rewrite machinery to clone the initializer expression on each use that would lifetime extend its temporaries.

---

Full diff: https://github.com/llvm/llvm-project/pull/87933.diff

9 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (-6)
• (modified) clang/lib/Sema/SemaExpr.cpp (+9-7)
• (modified) clang/lib/Sema/SemaInit.cpp (+27-12)
• (modified) clang/test/Analysis/lifetime-extended-regions.cpp (+1-1)
• (modified) clang/test/CXX/drs/dr16xx.cpp (-2)
• (modified) clang/test/CXX/drs/dr18xx.cpp (+1-4)
• (modified) clang/test/CXX/special/class.temporary/p6.cpp (+20)
• (modified) clang/test/SemaCXX/constexpr-default-arg.cpp (+2-2)
• (modified) clang/test/SemaCXX/eval-crashes.cpp (+2-4)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a1dda2d2461c31..ba779e83d2afd4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10010,12 +10010,6 @@ def warn_new_dangling_initializer_list : Warning<
   "the allocated initializer list}0 "
   "will be destroyed at the end of the full-expression">,
   InGroup<DanglingInitializerList>;
-def warn_unsupported_lifetime_extension : Warning<
-  "lifetime extension of "
-  "%select{temporary|backing array of initializer list}0 created "
-  "by aggregate initialization using a default member initializer "
-  "is not yet supported; lifetime of %select{temporary|backing array}0 "
-  "will end at the end of the full-expression">, InGroup<Dangling>;
 
 // For non-floating point, expressions of the form x == x or x != x
 // should result in a warning, since these always evaluate to a constant.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8db4fffeecfe35..b2e0f2a2a60113 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6338,10 +6338,9 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
         Res = Immediate.TransformInitializer(Param->getInit(),
                                              /*NotCopy=*/false);
       });
-      if (Res.isInvalid())
-        return ExprError();
-      Res = ConvertParamDefaultArgument(Param, Res.get(),
-                                        Res.get()->getBeginLoc());
+      if (Res.isUsable())
+        Res = ConvertParamDefaultArgument(Param, Res.get(),
+                                          Res.get()->getBeginLoc());
       if (Res.isInvalid())
         return ExprError();
       Init = Res.get();
@@ -6377,7 +6376,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
   Expr *Init = nullptr;
 
   bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
-
+  bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
   EnterExpressionEvaluationContext EvalContext(
       *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
 
@@ -6412,11 +6411,14 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
   ImmediateCallVisitor V(getASTContext());
   if (!NestedDefaultChecking)
     V.TraverseDecl(Field);
-  if (V.HasImmediateCalls) {
+  if (V.HasImmediateCalls || InLifetimeExtendingContext) {
     ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field,
                                                                    CurContext};
     ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
         NestedDefaultChecking;
+    // Pass down lifetime extending flag, and collect temporaries in
+    // CreateMaterializeTemporaryExpr when we rewrite the call argument.
+    keepInLifetimeExtendingContext();
 
     EnsureImmediateInvocationInDefaultArgs Immediate(*this);
     ExprResult Res;
@@ -6424,7 +6426,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
       Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
                                            /*CXXDirectInit=*/false);
     });
-    if (!Res.isInvalid())
+    if (Res.isUsable())
       Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
     if (Res.isInvalid()) {
       Field->setInvalidDecl();
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index a75e9925a43146..842412cd674d8c 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -710,6 +710,26 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
       if (VerifyOnly)
         return;
 
+      // Enter a lifetime extension context, then we can support lifetime
+      // extension of temporary created by aggregate initialization using a
+      // default member initializer (DR1815 https://wg21.link/CWG1815).
+      //
+      // In a lifetime extension context, BuildCXXDefaultInitExpr will clone the
+      // initializer expression on each use that would lifetime extend its
+      // temporaries.
+      EnterExpressionEvaluationContext LifetimeExtensionContext(
+          SemaRef, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+          /*LambdaContextDecl=*/nullptr,
+          Sema::ExpressionEvaluationContextRecord::EK_Other, true);
+
+      // Lifetime extension in default-member-init.
+      auto &LastRecord = SemaRef.ExprEvalContexts.back();
+
+      // Just copy previous record, make sure we haven't forget anything.
+      LastRecord =
+          SemaRef.ExprEvalContexts[SemaRef.ExprEvalContexts.size() - 2];
+      LastRecord.InLifetimeExtendingContext = true;
+
       ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
       if (DIE.isInvalid()) {
         hadError = true;
@@ -7699,6 +7719,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
     // 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)) {
+      assert(DIE->hasRewrittenInit() &&
+             "CXXDefaultInitExpr must has rewritten init");
       Path.push_back(
           {IndirectLocalPathEntry::DefaultInit, DIE, DIE->getField()});
       Init = DIE->getExpr();
@@ -8193,6 +8215,11 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
       }
 
       switch (shouldLifetimeExtendThroughPath(Path)) {
+      case PathLifetimeKind::ShouldExtend:
+        // We're supposed to lifetime-extend the temporary along this path (per
+        // the resolution of DR1815), we supported that by clone the initializer
+        // expression on each use that would lifetime extend its temporaries.
+        [[fallthrough]];
       case PathLifetimeKind::Extend:
         // Update the storage duration of the materialized temporary.
         // FIXME: Rebuild the expression instead of mutating it.
@@ -8200,18 +8227,6 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
                               ExtendingEntity->allocateManglingNumber());
         // Also visit the temporaries lifetime-extended by this initializer.
         return true;
-
-      case PathLifetimeKind::ShouldExtend:
-        // We're supposed to lifetime-extend the temporary along this path (per
-        // the resolution of DR1815), but we don't support that yet.
-        //
-        // FIXME: Properly handle this situation. Perhaps the easiest approach
-        // would be to clone the initializer expression on each use that would
-        // lifetime extend its temporaries.
-        Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
-            << RK << DiagRange;
-        break;
-
       case PathLifetimeKind::NoExtend:
         // If the path goes through the initialization of a variable or field,
         // it can't possibly reach a temporary created in this full-expression.
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4e98bd4b0403eb..8df9b8a54a7e02 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -122,7 +122,7 @@ void aggregateWithReferences() {
   clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
 
   // clang does not currently implement extending lifetime of object bound to reference members of aggregates,
-  // that are created from default member initializer (see `warn_unsupported_lifetime_extension` from `-Wdangling`)
+  // that are created from default member initializer (see `warn_unsupported_lifetime_extension` from `-Wdangling`) (Supported now).
   RefAggregate defaultInitExtended{i}; // clang-bug does not extend `Composite`
   clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
 }
diff --git a/clang/test/CXX/drs/dr16xx.cpp b/clang/test/CXX/drs/dr16xx.cpp
index f4d6c04fb8e073..52002a7d0db002 100644
--- a/clang/test/CXX/drs/dr16xx.cpp
+++ b/clang/test/CXX/drs/dr16xx.cpp
@@ -484,8 +484,6 @@ namespace dr1696 { // dr1696: 7
     const A &a = A(); // #dr1696-D1-a
   };
   D1 d1 = {}; // #dr1696-d1
-  // since-cxx14-warning@-1 {{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported; lifetime of temporary will end at the end of the full-expression}}
-  //   since-cxx14-note@#dr1696-D1-a {{initializing field 'a' with default member initializer}}
 
   struct D2 {
     const A &a = A(); // #dr1696-D2-a
diff --git a/clang/test/CXX/drs/dr18xx.cpp b/clang/test/CXX/drs/dr18xx.cpp
index e78730e8992cf8..52a90ea3a05d26 100644
--- a/clang/test/CXX/drs/dr18xx.cpp
+++ b/clang/test/CXX/drs/dr18xx.cpp
@@ -206,13 +206,10 @@ namespace dr1814 { // dr1814: yes
 #endif
 }
 
-namespace dr1815 { // dr1815: no
+namespace dr1815 { // dr1815: yes
 #if __cplusplus >= 201402L
-  // FIXME: needs codegen test
   struct A { int &&r = 0; }; // #dr1815-A 
   A a = {};
-  // since-cxx14-warning@-1 {{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported; lifetime of temporary will end at the end of the full-expression}} FIXME
-  //   since-cxx14-note@#dr1815-A {{initializing field 'r' with default member initializer}}
 
   struct B { int &&r = 0; }; // #dr1815-B
   // since-cxx14-error@-1 {{reference member 'r' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
diff --git a/clang/test/CXX/special/class.temporary/p6.cpp b/clang/test/CXX/special/class.temporary/p6.cpp
index 5554363cc69abb..002bf3e1c26740 100644
--- a/clang/test/CXX/special/class.temporary/p6.cpp
+++ b/clang/test/CXX/special/class.temporary/p6.cpp
@@ -269,6 +269,26 @@ void init_capture_init_list() {
   // CHECK: }
 }
 
+void check_dr1815() { // dr1815: yes
+#if __cplusplus >= 201402L
+
+  struct A {
+    int &&r = 0;
+    ~A() {}
+  };
+
+  struct B {
+    A &&a = A{};
+    ~B() {}
+  };
+
+  // CHECK: void @_Z12check_dr1815v()
+  // CHECK: call void @_ZZ12check_dr1815vEN1BD1Ev(
+  // CHECK: call void @_ZZ12check_dr1815vEN1AD1Ev(
+  B a = {};
+#endif
+}
+
 namespace P2718R0 {
 namespace basic {
 template <typename E> using T2 = std::list<E>;
diff --git a/clang/test/SemaCXX/constexpr-default-arg.cpp b/clang/test/SemaCXX/constexpr-default-arg.cpp
index 7c883692829548..8510a6ddc80399 100644
--- a/clang/test/SemaCXX/constexpr-default-arg.cpp
+++ b/clang/test/SemaCXX/constexpr-default-arg.cpp
@@ -32,8 +32,8 @@ void test_default_arg2() {
 }
 
 // Check that multiple CXXDefaultInitExprs don't cause an assertion failure.
-struct A { int &&r = 0; }; // expected-note 2{{default member initializer}}
+struct A { int &&r = 0; };
 struct B { A x, y; };
-B b = {}; // expected-warning 2{{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported}}
+B b = {}; // expected-no-diagnostics
 
 }
diff --git a/clang/test/SemaCXX/eval-crashes.cpp b/clang/test/SemaCXX/eval-crashes.cpp
index 017df977b26b7b..a06f60f71e9c7e 100644
--- a/clang/test/SemaCXX/eval-crashes.cpp
+++ b/clang/test/SemaCXX/eval-crashes.cpp
@@ -25,11 +25,9 @@ namespace pr33140_0b {
 }
 
 namespace pr33140_2 {
-  // FIXME: The declaration of 'b' below should lifetime-extend two int
-  // temporaries.
-  struct A { int &&r = 0; }; // expected-note 2{{initializing field 'r' with default member initializer}}
+  struct A { int &&r = 0; };
   struct B { A x, y; };
-  B b = {}; // expected-warning 2{{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported}}
+  B b = {};
 }
 
 namespace pr33140_3 {

[yronglin]

friendly ping~

[yronglin]

friendly ping~

[cor3ntin]

@hubert-reinterpretcast ping!

[yronglin]

Friendly ping! @zygoloid @hubert-reinterpretcast

[yronglin]

friendly ping~

[zygoloid]

Thanks for working on this.

[zygoloid]

If the bug is fixed, does the analyzer output not change here? In any case, the comment should explain whatever the new behavior is, and shouldn't be mentioning a warning that no longer exists.

[yronglin]

Thanks, good catch! we need update this.

[yronglin]

Please give me some time to investigate, I'm not very familiar with analyzer.

[zygoloid]

Suggested change

	<td class="full" align="center">Clang 19</td>
	<td class="unreleased" align="center">Clang 19</td>

We use a different CSS class here for features that haven't been in an official release yet.

[yronglin]

Thanks for your tips!

[yronglin]

Thanks for you review!

[Endilll]

@ZequanWu thank you for your help, but next time you should disable renaming passes as specified in #89807 (comment). C-Reduce output can be typically reduced further manually, but it's a pain without names.

[yronglin]

Thanks, I'll take a look now.

[yronglin]

I've a more short reproducer:

namespace std {
template <class T> class initializer_list {};
}

template <typename T, int> class C {
public:
  C(std::initializer_list<T>);
};

template <typename T> using Ptr =__remove_pointer(T) *;
template <typename T>  C(T) ->  C<Ptr<T>, sizeof(T)>;

class A {
public:
  template <typename T1, typename T2>
  T1 *some_func(T2 &&);
};

struct B : A {
  int *ar = some_func<int>(C{some_func<int>(0)});
  B() {}
};

[ZequanWu]

Great, I just finished a creduce run with renaming pass disabled, but yours is short.

[yronglin]

I've debug in local. The crash issue caused by initializer rebuild failed in

llvm-project/clang/lib/Sema/SemaExpr.cpp

Lines 5717 to 5721 in 8e00703

	SFINAETrap Trap(*this);
	runWithSufficientStackSpace(Loc, [&] {
	Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
	/*CXXDirectInit=*/false);
	});

, if remove Line 5717 SFINAETrap Trap(*this);, we can got diagnostics like the following:

./main.cpp:23:28: error: no viable constructor or deduction guide for deduction of template arguments of 'C'
   23 |   int *ar = some_func<int>(C{some_func<int>(0)});
      |                            ^
./main.cpp:6:34: note: candidate template ignored: couldn't infer template argument 'T'
    6 | template <typename T, int> class C {
      |                                  ^
./main.cpp:8:3: note: candidate template ignored: couldn't infer template argument ''
    8 |   C(std::initializer_list<T>);
      |   ^
./main.cpp:12:24: note: candidate template ignored: couldn't infer template argument 'T'
   12 | template <typename T>  C(T) ->  C<Ptr<T>, sizeof(T)>;
      |                        ^
1 error generated.

But why does the check pass during Parse phase but fails when we rebuilding CXXDefaultInitExpr? Through tracing, I found that the root cause was the parameter bool ListInitialization that passed to RebuildCXXTemporaryObjectExpr(fall throuth to Sema::BuildCXXTypeConstructExpr). During parsing, ListInitialization was true, but it became false during rebuilding, it's cause InitializationKind to become DirectInit instead of DirectListInit. Finally, causing Sema::DeduceTemplateSpecializationFromInitializer fail.

llvm-project/clang/lib/Sema/TreeTransform.h

Lines 14116 to 14121 in 8e00703

	// FIXME: We should just pass E->isListInitialization(), but we're not
	// prepared to handle list-initialization without a child InitListExpr.
	SourceLocation LParenLoc = T->getTypeLoc().getEndLoc();
	return getDerived().RebuildCXXTemporaryObjectExpr(
	T, LParenLoc, Args, E->getEndLoc(),
	/*ListInitialization=*/LParenLoc.isInvalid());

Therefore, I think the key to the problem is to fix TreeTransform.h:14116's FIXME. As the comments in TreeTransform.h:14116 said, we should pass E->isListInitialization(), because E is actually list initialization, I have tried this modification, but it will cause 3 lit failures. We have not try to rebuild the CXXDefaultInitExpr before this PR, so it's works fine. I'll try to continue investigating this issue!

[ZequanWu]

Can we revert this change while you are investigating if this fix is not trivial?

[ZequanWu]

Here's a smaller repro of the -Wuninitialized warning:

int TestBody_got;
namespace std {
template <int __v> struct integral_constant {
  static const int value = __v;
};
template <bool, class _Tp> using enable_if_t = _Tp;
template <class> class initializer_list {};
template <typename, typename>
constexpr bool IsTypeOrDerived = integral_constant<__is_same(int, int)>::value;
template <bool CONDITION, typename T>
using EnableIf = enable_if_t<CONDITION, T>;
template <typename T, typename BASE>
using EnableIfIsType = EnableIf<IsTypeOrDerived<T, BASE>, T>;
template <int> class Vector {
public:
  Vector(initializer_list<int>);
};
template <typename... Ts> Vector(Ts...) -> Vector<sizeof...(Ts)>;
class ProgramBuilder {
public:
  template <typename T, typename ARGS> EnableIfIsType<T, int> *create(ARGS);
};
using TestHelper = ProgramBuilder;
struct TypeTest : TestHelper {
  int *str_f16 = create<int>(Vector{0});
  TypeTest() {}
};
class TypeTest_Element_Test : TypeTest {
  void TestBody();
};
}
void std::TypeTest_Element_Test::TestBody() {
  int *expect = str_f16;
  &TestBody_got != expect;
}
$ clang reduce.cpp -std=c++20 -fsyntax-only -Wuninitialized -Wno-unused-comparison
reduce.cpp:34:20: warning: variable 'expect' is uninitialized when used here [-Wuninitialized]
   34 |   &TestBody_got != expect;
      |                    ^~~~~~
reduce.cpp:33:14: note: initialize the variable 'expect' to silence this warning
   33 |   int *expect = str_f16;
      |              ^
      |               = nullptr
1 warning generated.

Can you verify if this is a false positive or not? If it's a false positive and would take a while to fix, can you revert this commit?

Update:
Manually reduce further:

int TestBody_got;
namespace std {
template <class> class initializer_list {};
template <int> class Vector {
public:
  Vector(initializer_list<int>);
};
template <typename... Ts> Vector(Ts...) -> Vector<sizeof...(Ts)>;
class ProgramBuilder {
public:
  template <typename T, typename ARGS> int *create(ARGS);
};

struct TypeTest : ProgramBuilder {
  int *str_f16 = create<int>(Vector{0});
  TypeTest() {}
};
class TypeTest_Element_Test : TypeTest {
  void TestBody();
};
}
void std::TypeTest_Element_Test::TestBody() {
  int *expect = str_f16;
  &TestBody_got != expect;
}

[erichkeane]

I think we have enough evidence that we have to revert this, so I've done so in 224116a

@yronglin : Please submit a PR that includes the original patch + whatever it takes to fix the above issues. Unfortunately reverts happen :/ But I definitely appreciate the work you're doing on this.

[yronglin]

Sorry for the late reply, I found the update of comments from the email, and I have a new PR that fixes these problems.

[yronglin]

@erichkeane Can you please take a look?

[jyknight]

Here's a test case that clang diagnoses with -Wundefined-inline after this patch, which I'm not sure whether is correct or not. It might violate https://eel.is/c++draft/temp.inst#11 to attempt to instantiate the unused S::operator int<int>?

I'm having a hard time telling whether this code is ill-defined NDR, or if the compiler is required to accept this.

Clang stopped diagnosing this code with the fix for CWG2631, ca61961. Now this PR makes it diagnose again, despite that the default arg is never actually used.

struct S {
  template <typename T>
  consteval operator T();
};

struct M {
  int x = S();
};

void test(M m = {}) {}

test2.cc:3:13: error: inline function 'S::operator int<int>' is not defined [-Werror,-Wundefined-inline]
    3 |   consteval operator T();
      |             ^
test2.cc:7:11: note: used here
    7 |   int x = S();
      |           ^
1 error generated.

[Endilll]

It might violate https://eel.is/c++draft/temp.inst#11 to attempt to instantiate the unused S::operator int?

I don't think this clause of the standard is violated: you need to instantiate a declaration of the conversion function to at the end of definition of M, otherwise you can't "determine the correctness of the default member initializer" in int x = S();.

[jyknight]

We don't diagnose it at the end of the definition of M -- it looks like we explicitly intentionally stopped doing so in the commit I referenced. That's why I'm a little confused here.

With this patch, we start to diagnose only in the final line of code, void test(M m = {}) {} (that line doesn't appear in the error message, but it's the trigger).

[yronglin]

Thanks for the new test case, it's fixed with #92527

[erichkeane]

@erichkeane Can you please take a look?

Looking now, I was out on Friday due to illness.

[yronglin]

@erichkeane Can you please take a look?

Looking now, I was out on Friday due to illness.

I'm sorry to hear that. Sending all my best and wishing you a rapid comeback.