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

yronglin · 2024-05-17T11:30:57Z

This PR reapply #87933

llvmbot · 2024-05-17T11:31:26Z

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

This PR reapply #87933

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

15 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (-6)
(modified) clang/lib/Sema/SemaExpr.cpp (+23-8)
(modified) clang/lib/Sema/SemaExprCXX.cpp (-3)
(modified) clang/lib/Sema/SemaInit.cpp (+1-18)
(modified) clang/lib/Sema/TreeTransform.h (+8-4)
(modified) clang/test/AST/ast-dump-default-init-json.cpp (+3-3)
(modified) clang/test/AST/ast-dump-default-init.cpp (+1-1)
(modified) clang/test/Analysis/lifetime-extended-regions.cpp (+5-4)
(modified) clang/test/CXX/drs/cwg16xx.cpp (-2)
(modified) clang/test/CXX/drs/cwg18xx.cpp (+17-8)
(modified) clang/test/CXX/special/class.temporary/p6.cpp (+34)
(modified) clang/test/SemaCXX/constexpr-default-arg.cpp (+2-2)
(modified) clang/test/SemaCXX/cxx11-default-member-initializers.cpp (+74)
(modified) clang/test/SemaCXX/eval-crashes.cpp (+2-4)
(modified) clang/www/cxx_dr_status.html (+1-1)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9372c862a36cb..9680efc4951aa 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10041,12 +10041,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 e0aae6333e1a1..95ed15b9c2928 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5621,10 +5621,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();
@@ -5660,7 +5659,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
   Expr *Init = nullptr;
 
   bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
-
+  bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
   EnterExpressionEvaluationContext EvalContext(
       *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
 
@@ -5695,19 +5694,35 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
   ImmediateCallVisitor V(getASTContext());
   if (!NestedDefaultChecking)
     V.TraverseDecl(Field);
-  if (V.HasImmediateCalls) {
+
+  // CWG1815
+  // Support lifetime extension of temporary created by aggregate
+  // initialization using a default member initializer. We should always rebuild
+  // the initializer if it contains any temporaries (if the initializer
+  // expression is an ExprWithCleanups). Then make sure the normal lifetime
+  // extension code recurses into the default initializer and does lifetime
+  // extension when warranted.
+  bool ContainsAnyTemporaries =
+      isa_and_present<ExprWithCleanups>(Field->getInClassInitializer());
+  if (V.HasImmediateCalls || InLifetimeExtendingContext ||
+      ContainsAnyTemporaries) {
     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;
+
+    // Rebuild CXXDefaultInitExpr might cause diagnostics.
+    SFINAETrap Trap(*this);
     runWithSufficientStackSpace(Loc, [&] {
       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/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e4601f7d6c47d..ef2c07fe011ca 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1554,9 +1554,6 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
                                 bool ListInitialization) {
   QualType Ty = TInfo->getType();
   SourceLocation TyBeginLoc = TInfo->getTypeLoc().getBeginLoc();
-
-  assert((!ListInitialization || Exprs.size() == 1) &&
-         "List initialization must have exactly one expression.");
   SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);
 
   InitializedEntity Entity =
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 2177972f3af2c..708286e192f9c 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -8066,11 +8066,6 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
 enum PathLifetimeKind {
   /// Lifetime-extend along this path.
   Extend,
-  /// We should lifetime-extend, but we don't because (due to technical
-  /// limitations) we can't. This happens for default member initializers,
-  /// which we don't clone for every use, so we don't have a unique
-  /// MaterializeTemporaryExpr to update.
-  ShouldExtend,
   /// Do not lifetime extend along this path.
   NoExtend
 };
@@ -8082,7 +8077,7 @@ shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
   PathLifetimeKind Kind = PathLifetimeKind::Extend;
   for (auto Elem : Path) {
     if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
-      Kind = PathLifetimeKind::ShouldExtend;
+      Kind = PathLifetimeKind::Extend;
     else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
       return PathLifetimeKind::NoExtend;
   }
@@ -8202,18 +8197,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/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index b10e5ba65eb1c..71010d4ce86eb 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14102,6 +14102,13 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
     if (TransformExprs(E->getArgs(), E->getNumArgs(), true, Args,
                        &ArgumentChanged))
       return ExprError();
+
+    if (E->isListInitialization() && !E->isStdInitListInitialization()) {
+      ExprResult Res = RebuildInitList(E->getBeginLoc(), Args, E->getEndLoc());
+      if (Res.isInvalid())
+        return ExprError();
+      Args = {Res.get()};
+    }
   }
 
   if (!getDerived().AlwaysRebuild() &&
@@ -14113,12 +14120,9 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
     return SemaRef.MaybeBindToTemporary(E);
   }
 
-  // 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());
+      T, LParenLoc, Args, E->getEndLoc(), E->isListInitialization());
 }
 
 template<typename Derived>
diff --git a/clang/test/AST/ast-dump-default-init-json.cpp b/clang/test/AST/ast-dump-default-init-json.cpp
index 1058b4e3ea4d9..f4949a9c9eedf 100644
--- a/clang/test/AST/ast-dump-default-init-json.cpp
+++ b/clang/test/AST/ast-dump-default-init-json.cpp
@@ -789,10 +789,10 @@ void test() {
 // CHECK-NEXT:                  "valueCategory": "lvalue",
 // CHECK-NEXT:                  "extendingDecl": {
 // CHECK-NEXT:                   "id": "0x{{.*}}",
-// CHECK-NEXT:                   "kind": "FieldDecl",
-// CHECK-NEXT:                   "name": "a",
+// CHECK-NEXT:                   "kind": "VarDecl",
+// CHECK-NEXT:                   "name": "b",
 // CHECK-NEXT:                   "type": {
-// CHECK-NEXT:                    "qualType": "const A &"
+// CHECK-NEXT:                    "qualType": "B"
 // CHECK-NEXT:                   }
 // CHECK-NEXT:                  },
 // CHECK-NEXT:                  "storageDuration": "automatic",
diff --git a/clang/test/AST/ast-dump-default-init.cpp b/clang/test/AST/ast-dump-default-init.cpp
index 15b29f04bf21b..26864fbf15424 100644
--- a/clang/test/AST/ast-dump-default-init.cpp
+++ b/clang/test/AST/ast-dump-default-init.cpp
@@ -13,7 +13,7 @@ void test() {
 }
 // CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue has rewritten init
 // CHECK-NEXT:  `-ExprWithCleanups 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue
-// CHECK-NEXT:    `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Field 0x{{[^ ]*}} 'a' 'const A &'
+// CHECK-NEXT:    `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Var 0x{{[^ ]*}} 'b' 'B'
 // CHECK-NEXT:      `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp>
 // CHECK-NEXT:        `-CXXFunctionalCastExpr 0x{{[^ ]*}} <{{.*}}> 'A' functional cast to A <NoOp>
 // CHECK-NEXT:          `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'A'
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4e98bd4b0403e..4458ad294af7c 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -120,10 +120,11 @@ void aggregateWithReferences() {
   clang_analyzer_dump(viaReference);    // expected-warning-re {{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
   clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
   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`)
-  RefAggregate defaultInitExtended{i}; // clang-bug does not extend `Composite`
+  
+  // FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
+  // that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
+  // The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
+  RefAggregate defaultInitExtended{i};
   clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
 }
 
diff --git a/clang/test/CXX/drs/cwg16xx.cpp b/clang/test/CXX/drs/cwg16xx.cpp
index cf6b45ceabf2c..82ef871939d2c 100644
--- a/clang/test/CXX/drs/cwg16xx.cpp
+++ b/clang/test/CXX/drs/cwg16xx.cpp
@@ -483,8 +483,6 @@ namespace cwg1696 { // cwg1696: 7
     const A &a = A(); // #cwg1696-D1-a
   };
   D1 d1 = {}; // #cwg1696-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@#cwg1696-D1-a {{initializing field 'a' with default member initializer}}
 
   struct D2 {
     const A &a = A(); // #cwg1696-D2-a
diff --git a/clang/test/CXX/drs/cwg18xx.cpp b/clang/test/CXX/drs/cwg18xx.cpp
index 35615076a6288..89adc28384904 100644
--- a/clang/test/CXX/drs/cwg18xx.cpp
+++ b/clang/test/CXX/drs/cwg18xx.cpp
@@ -56,7 +56,7 @@ namespace cwg1804 { // cwg1804: 2.7
 template <typename, typename>
 struct A {
   void f1();
-
+  
   template <typename V>
   void f2(V);
 
@@ -73,7 +73,7 @@ struct A {
 template <typename U>
 struct A<int, U> {
   void f1();
-
+  
   template <typename V>
   void f2(V);
 
@@ -97,7 +97,7 @@ class D {
 template <typename U>
 struct A<double, U> {
   void f1();
-
+  
   template <typename V>
   void f2(V);
 
@@ -206,19 +206,28 @@ namespace cwg1814 { // cwg1814: yes
 #endif
 }
 
-namespace cwg1815 { // cwg1815: no
+namespace cwg1815 { // cwg1815: 19
 #if __cplusplus >= 201402L
-  // FIXME: needs codegen test
-  struct A { int &&r = 0; }; // #cwg1815-A
+  struct A { int &&r = 0; };
   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@#cwg1815-A {{initializing field 'r' with default member initializer}}
 
   struct B { int &&r = 0; }; // #cwg1815-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}}
   //   since-cxx14-note@#cwg1815-B {{initializing field 'r' with default member initializer}}
   //   since-cxx14-note@#cwg1815-b {{in implicit default constructor for 'cwg1815::B' first required here}}
   B b; // #cwg1815-b
+
+#if __cplusplus >= 201703L
+  struct C { const int &r = 0; };
+  constexpr C c = {}; // OK, since cwg1815
+  static_assert(c.r == 0);
+
+  constexpr int f() {
+    A a = {}; // OK, since cwg1815
+    return a.r;
+  }
+  static_assert(f() == 0);
+#endif
 #endif
 }
 
diff --git a/clang/test/CXX/special/class.temporary/p6.cpp b/clang/test/CXX/special/class.temporary/p6.cpp
index 5554363cc69ab..a6d2adfd1fd2c 100644
--- a/clang/test/CXX/special/class.temporary/p6.cpp
+++ b/clang/test/CXX/special/class.temporary/p6.cpp
@@ -269,6 +269,40 @@ 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() {}
+  };
+  B a = {};
+  
+  // CHECK: call {{.*}}block_scope_begin_function
+  extern void block_scope_begin_function();
+  extern void block_scope_end_function();
+  block_scope_begin_function();
+  {
+    // CHECK: call void @_ZZ12check_dr1815vEN1BD1Ev
+    // CHECK: call void @_ZZ12check_dr1815vEN1AD1Ev
+    B b = {};
+  }
+  // CHECK: call {{.*}}block_scope_end_function
+  block_scope_end_function();
+
+  // CHECK: call {{.*}}some_other_function
+  extern void some_other_function();
+  some_other_function();
+  // CHECK: call void @_ZZ12check_dr1815vEN1BD1Ev
+  // CHECK: call void @_ZZ12check_dr1815vEN1AD1Ev
+#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 ec9b2927880bd..901123bfb359f 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/cxx11-default-member-initializers.cpp b/clang/test/SemaCXX/cxx11-default-member-initializers.cpp
index dd8e9c6b7fc11..1ea8b98cd8636 100644
--- a/clang/test/SemaCXX/cxx11-default-member-initializers.cpp
+++ b/clang/test/SemaCXX/cxx11-default-member-initializers.cpp
@@ -27,6 +27,80 @@ class MemInit {
   C m = s;
 };
 
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+
+// libc++'s implementation
+template <class _E> class initializer_list {
+  const _E *__begin_;
+  size_t __size_;
+
+  initializer_list(const _E *__b, size_t __s) : __begin_(__b), __size_(__s) {}
+
+public:
+  typedef _E value_type;
+  typedef const _E &reference;
+  typedef const _E &const_reference;
+  typedef size_t size_type;
+
+  typedef const _E *iterator;
+  typedef const _E *const_iterator;
+
+  initializer_list() : __begin_(nullptr), __size_(0) {}
+
+  size_t size() const { return __size_; }
+  const _E *begin() const { return __begin_; }
+  const _E *end() const { return __begin_ + __size_; }
+};
+} // namespace std
+
+#if __cplusplus >= 201703L
+namespace test_rebuild {
+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 {
+  // Test CXXDefaultInitExpr rebuild issue in 
+  // https://github.com/llvm/llvm-project/pull/87933
+  int *ar = some_func<int>(C{some_func<int>(0)});
+  B() {}
+};
+
+int TestBody_got;
+template <int> class Vector {
+public:
+  Vector(std::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 TypeTest_Element_Test::TestBody() {
+  int *expect = str_f16;
+  &TestBody_got != expect; // expected-warning {{inequality comparison result unused}}
+}
+} //  namespace test_rebuild
+#endif // __cplusplus >= 201703L
+
 #if __cplusplus >= 202002L
 // This test ensures cleanup expressions are correctly produced
 // in the presence of default member initializers.
diff --git a/clang/test/SemaCXX/eval-crashes.cpp b/clang/test/SemaCXX/eval-crashes.cpp
index 017df977b26b7..a06f60f71e9c7 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 {
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 2ed79fa21db26..abf5d4ae4676d 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -10698,7 +10698,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/1815.html">1815</a></td>
     <td>CD4</td>
     <td>Lifetime extension in aggregate initialization</td>
-    <td class="none" align="center">No</td>
+    <td class="unreleased" align="center">Clang 19</td>
   </tr>
   <tr id="1816">
     <td><a href="https://cplusplus.github.io/CWG/issues/1816.html">1816</a></td>

yronglin · 2024-05-17T11:36:42Z

clang/lib/Sema/SemaExpr.cpp

    EnsureImmediateInvocationInDefaultArgs Immediate(*this);
    ExprResult Res;
+
+    // Rebuild CXXDefaultInitExpr might cause diagnostics.
+    SFINAETrap Trap(*this);


Should we suppress diagnostic messages? Perhaps the initializer with errors should not be rebuilt.

Is there a way to detect if there is an error in the initializer? For example, access errors.

I'm not sure I get the question? The SFINAETrap IS suppressing at least some diagnostics.

We can see if there are already errors pre-transformation via containsError (or if it is nullptr, depending on RecoveryExpr). So if there was previously detected error, one of those should catch it.

Thanks for your review! Here we add SFINAETrap Trap(*this). It was used to resolve the issue which looks like https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/dcl.decl/dcl.init/p14-0x.cpp .
A shorter reproducer:

class Private { Private(int); // expected-note {{here}} public: Private(); }; class S { Private p = 42; // expected-error {{private constructor}} S() {} // expected-error {{call to deleted constructor of 'NoDefault'}} \ expected-error {{must explicitly initialize the member 'e1' which does not have a default constructor}} S(int) {} }; // FIXME: test the other forms which use copy-initialization

Field->getInClassInitializer() contains an bad access error, then we try to rebuild this initializer, the diagnostic will emit 3 times. (The result of Field->getInClassInitializer()->containsErrors() 3 times are false).
1st time: parser member initializer.
2nd time: After rebuild, Line 5726 Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
3rd time: After rebuild, Line 5726 Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);

./error.cpp:8:15: error: field of type 'Private' has private constructor 8 | Private p = 42; // expected-error {{private constructor}} | ^ ./error.cpp:2:3: note: implicitly declared private here 2 | Private(int); // expected-note {{here}} | ^ ./error.cpp:8:15: error: field of type 'Private' has private constructor 8 | Private p = 42; // expected-error {{private constructor}} | ^ ./error.cpp:2:3: note: implicitly declared private here 2 | Private(int); // expected-note {{here}} | ^ ./error.cpp:8:15: error: field of type 'Private' has private constructor 8 | Private p = 42; // expected-error {{private constructor}} | ^ ./error.cpp:2:3: note: implicitly declared private here 2 | Private(int); // expected-note {{here}} | ^ 3 errors generated.

…ggregate initialization using a default member initializer Signed-off-by: yronglin <yronglin777@gmail.com>

yronglin · 2024-05-17T11:47:07Z

clang/test/SemaCXX/cxx11-default-member-initializers.cpp

+} // namespace std
+
+#if __cplusplus >= 201703L
+namespace test_rebuild {


Add more test that from #87933 's comments.

yronglin · 2024-05-17T11:48:19Z

clang/lib/Sema/TreeTransform.h

  SourceLocation LParenLoc = T->getTypeLoc().getEndLoc();
  return getDerived().RebuildCXXTemporaryObjectExpr(
-      T, LParenLoc, Args, E->getEndLoc(),
-      /*ListInitialization=*/LParenLoc.isInvalid());
+      T, LParenLoc, Args, E->getEndLoc(), E->isListInitialization());


The issue in #87933 caused by /*ListInitialization=*/LParenLoc.isInvalid().

yronglin · 2024-05-17T11:53:54Z

clang/lib/Sema/TreeTransform.h

@@ -14102,6 +14102,13 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
    if (TransformExprs(E->getArgs(), E->getNumArgs(), true, Args,
                       &ArgumentChanged))
      return ExprError();
+
+    if (E->isListInitialization() && !E->isStdInitListInitialization()) {


If it is list initialization, but the constructor is called, and it is not the case of std::initializer_list, we should recreate an InitListExpr. https://godbolt.org/z/K4rzxosx3

erichkeane

Not the best one to be reviewing this, but the code generally looks good to me.

erichkeane · 2024-05-20T14:27:29Z

clang/lib/Sema/SemaExpr.cpp

    EnsureImmediateInvocationInDefaultArgs Immediate(*this);
    ExprResult Res;
+
+    // Rebuild CXXDefaultInitExpr might cause diagnostics.
+    SFINAETrap Trap(*this);


I'm not sure I get the question? The SFINAETrap IS suppressing at least some diagnostics.

We can see if there are already errors pre-transformation via containsError (or if it is nullptr, depending on RecoveryExpr). So if there was previously detected error, one of those should catch it.

erichkeane

I think this looks ok, I'd love it if @cor3ntin took another look, but I'm as happy as before.

yronglin · 2024-05-21T15:17:44Z

I think this looks ok, I'd love it if @cor3ntin took another look, but I'm as happy as before.

Thanks for your review!

yronglin · 2024-05-22T23:59:32Z

Thanks for the review!

… expression (#91879) Depends on #92527 Clang now support the following: - Extending lifetime of object bound to reference members of aggregates, that are created from default member initializer. - Rebuild `CXXDefaultArgExpr` and `CXXDefaultInitExpr` as needed where called or constructed. But CFG and ExprEngine need to be updated to address this change. This PR add `CXXDefaultArgExpr` and `CXXDefaultInitExpr` into CFG, and correct handle these expressions in ExprEngine --------- Signed-off-by: yronglin <yronglin777@gmail.com>

bgra8 · 2024-06-03T10:20:28Z

heads-up @yronglin: we've bisected many compilation breakages to this patch.
We're working on a reproducer.

bgra8 · 2024-06-05T07:51:08Z

Here's the reproducer:

struct a {
  virtual int *b();
};
template <class c> struct d : a {
  int *b() { new c; }
};
int *e(a *);
struct f {
  char *g;
};
struct h {};
struct i {
  i(const f &);
  i(h);
};
struct l {
  i j = i({.g = ""});
};
int *k = e(new d<l>);
struct n : l {};
int *m = e(new d<n>);

Compilation command:

$ clang -c repro.cc

This compiles fine with clang-18, trunk gcc and previous clang revision.

@yronglin can you please have a look?

…ult init expression (#91879)" (#94597) This depends on #92527 which needs to be reverted due to #92527 (comment). This reverts commit 905b402. Co-authored-by: Bogdan Graur <bgraur@google.com>

…rary created by aggregate initialization using a default member initializer" (llvm#92527)" Reverting due to llvm#92527 (comment). This reverts commit f049d72.

…rary created by aggregate initialization using a default member initializer" (#92527)" (#94600) Reverting due to #92527 (comment). This reverts commit f049d72. Co-authored-by: Bogdan Graur <bgraur@google.com>

alexfh · 2024-06-06T10:02:27Z

Here's the reproducer:

struct a {
  virtual int *b();
};
template <class c> struct d : a {
  int *b() { new c; }
};
int *e(a *);
struct f {
  char *g;
};
struct h {};
struct i {
  i(const f &);
  i(h);
};
struct l {
  i j = i({.g = ""});
};
int *k = e(new d<l>);
struct n : l {};
int *m = e(new d<n>);

A bit clearer repro: https://gcc.godbolt.org/z/r9EdcoW8G

The new clang behavior seems wrong to me.

bgra8 · 2024-06-06T10:04:05Z

I reverted this and the dependent patch based on #92527 (comment) to bring back the repo back to green.

yronglin · 2024-07-19T14:58:39Z

@bgra8 @alexfh Thanks report this issue, I've a new PR to fix it, and added your reproducer into the test.

…ated by aggregate initialization using a default member initializer" (#97308) The PR reapply #92527. Implemented CWG1815 and fixed the bugs mentioned in the comments of #92527 and #87933. The reason why the original PR was reverted was that errors might occur during the rebuild. --------- Signed-off-by: yronglin <yronglin777@gmail.com>

yronglin requested review from zygoloid, cor3ntin and erichkeane May 17, 2024 11:30

yronglin requested a review from Endilll as a code owner May 17, 2024 11:30

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 17, 2024

yronglin requested a review from ZequanWu May 17, 2024 11:35

yronglin commented May 17, 2024

View reviewed changes

[Clang][CWG1815] Support lifetime extension of temporary created by a…

4b90d24

…ggregate initialization using a default member initializer Signed-off-by: yronglin <yronglin777@gmail.com>

yronglin force-pushed the cwg1815 branch from 433df06 to 4b90d24 Compare May 17, 2024 11:39

yronglin commented May 17, 2024

View reviewed changes

yronglin mentioned this pull request May 17, 2024

[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression #91879

Merged

yronglin requested a review from AaronBallman May 17, 2024 17:21

This was referenced May 18, 2024

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

Merged

Clang crash on valid code since #87933 #92414

Closed

erichkeane reviewed May 20, 2024

View reviewed changes

erichkeane approved these changes May 21, 2024

View reviewed changes

cor3ntin approved these changes May 22, 2024

View reviewed changes

yronglin merged commit f049d72 into llvm:main May 22, 2024
4 checks passed

bgra8 mentioned this pull request Jun 6, 2024

Revert "[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (#91879)" #94597

Merged

bgra8 mentioned this pull request Jun 6, 2024

Revert "Reapply "[Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer" (#92527)" #94600

Merged

yronglin mentioned this pull request Jul 1, 2024

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

Merged

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

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

Uh oh!

Conversation

yronglin commented May 17, 2024

Uh oh!

llvmbot commented May 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yronglin May 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

yronglin commented May 21, 2024

Uh oh!

yronglin commented May 22, 2024

Uh oh!

Uh oh!

bgra8 commented Jun 3, 2024

Uh oh!

bgra8 commented Jun 5, 2024

Uh oh!

alexfh commented Jun 6, 2024

Uh oh!

bgra8 commented Jun 6, 2024

Uh oh!

yronglin commented Jul 19, 2024

Uh oh!

Uh oh!

yronglin May 20, 2024 •

edited

Loading