From: Richard Smith <richard@metafoo.co.uk>
Date: Fri, 15 Jan 2021 23:50:25 +0000 (-0800)
Subject: PR48763: Better handling for classes that inherit a default constructor.
X-Git-Tag: llvmorg-13-init~883
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=bc713f6a004723d1325bc16e1efc32d0ac82f939;p=platform%2Fupstream%2Fllvm.git

PR48763: Better handling for classes that inherit a default constructor.

The C++ standard wording doesn't appear to properly handle the case
where a class inherits a default constructor from a base class. Various
properties of classes are defined in terms of the corresponding property
of the default constructor, and in this case, the class does not have a
default constructor despite being default-constructible, which the
wording doesn't handle properly.

This change implements a tentative fix for these problems, which has
also been proposed to the C++ committee: if a class would inherit a
default constructor, and does not explicitly declare one, then one is
implicitly declared.
---

diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
index 4ce6771..d15d669 100644
--- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -131,6 +131,10 @@ FIELD(HasUninitializedFields, 1, NO_MERGE)
 /// constructors from a base class.
 FIELD(HasInheritedConstructor, 1, NO_MERGE)
 
+/// True if there are any member using-declarations that inherit
+/// default constructors from a base class.
+FIELD(HasInheritedDefaultConstructor, 1, NO_MERGE)
+
 /// True if there are any member using-declarations named
 /// 'operator='.
 FIELD(HasInheritedAssignment, 1, NO_MERGE)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 568eeb6..e32101b 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -744,9 +744,14 @@ public:
   ///
   /// This value is used for lazy creation of default constructors.
   bool needsImplicitDefaultConstructor() const {
-    return !data().UserDeclaredConstructor &&
-           !(data().DeclaredSpecialMembers & SMF_DefaultConstructor) &&
-           (!isLambda() || lambdaIsDefaultConstructibleAndAssignable());
+    return (!data().UserDeclaredConstructor &&
+            !(data().DeclaredSpecialMembers & SMF_DefaultConstructor) &&
+            (!isLambda() || lambdaIsDefaultConstructibleAndAssignable())) ||
+           // FIXME: Proposed fix to core wording issue: if a class inherits
+           // a default constructor and doesn't explicitly declare one, one
+           // is declared implicitly.
+           (data().HasInheritedDefaultConstructor &&
+            !(data().DeclaredSpecialMembers & SMF_DefaultConstructor));
   }
 
   /// Determine whether this class has any user-declared constructors.
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index b806adf..0368ada 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -81,7 +81,9 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false),
       HasOnlyCMembers(true), HasInClassInitializer(false),
       HasUninitializedReferenceMember(false), HasUninitializedFields(false),
-      HasInheritedConstructor(false), HasInheritedAssignment(false),
+      HasInheritedConstructor(false),
+      HasInheritedDefaultConstructor(false),
+      HasInheritedAssignment(false),
       NeedOverloadResolutionForCopyConstructor(false),
       NeedOverloadResolutionForMoveConstructor(false),
       NeedOverloadResolutionForCopyAssignment(false),
@@ -814,6 +816,8 @@ void CXXRecordDecl::addedMember(Decl *D) {
     //   constructor [...]
     if (Constructor->isConstexpr() && !Constructor->isCopyOrMoveConstructor())
       data().HasConstexprNonCopyMoveConstructor = true;
+    if (!isa<CXXConstructorDecl>(D) && Constructor->isDefaultConstructor())
+      data().HasInheritedDefaultConstructor = true;
   }
 
   // Handle destructors.
diff --git a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p15.cpp b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p15.cpp
index d64807e..4951ed6 100644
--- a/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p15.cpp
+++ b/clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p15.cpp
@@ -36,7 +36,7 @@ namespace default_ctor {
   };
 
   struct A {
-    A(); // expected-note {{candidate}}
+    A();
 
     A(C &&);
     C &operator=(C&&);
@@ -48,7 +48,7 @@ namespace default_ctor {
   };
 
   struct B {
-    B(); // expected-note {{candidate}}
+    B();
 
     B(C &&);
     C &operator=(C&&);
@@ -66,9 +66,9 @@ namespace default_ctor {
     using B::operator=;
   };
   struct D : A, B {
-    using A::A; // expected-note 2{{inherited here}}
+    using A::A; // expected-note {{inherited here}}
     using A::operator=;
-    using B::B; // expected-note 2{{inherited here}}
+    using B::B; // expected-note {{inherited here}}
     using B::operator=;
 
     D(int);
@@ -84,7 +84,9 @@ namespace default_ctor {
 
   // D does not declare D(), D(D&&), nor operator=(D&&), so the base class
   // versions are inherited.
-  D d; // expected-error {{ambiguous}}
+  // Exception: we implicitly declare a default constructor so that we can
+  // reason about its properties.
+  D d; // ok, implicitly declared
   void f(D d) {
     D d2(static_cast<D&&>(d)); // ok, ignores inherited constructors
     D d3(convert_to_D1{}); // ok, ignores inherited constructors
diff --git a/clang/test/CXX/special/class.ctor/p6-0x.cpp b/clang/test/CXX/special/class.ctor/p6-0x.cpp
index 9860317..156a2b2 100644
--- a/clang/test/CXX/special/class.ctor/p6-0x.cpp
+++ b/clang/test/CXX/special/class.ctor/p6-0x.cpp
@@ -94,3 +94,34 @@ namespace UnionCtors {
     friend constexpr E::E() noexcept; // expected-error {{follows non-constexpr declaration}}
   };
 }
+
+namespace PR48763 {
+  // FIXME: We implement a speculative wording fix here: if a class inherits a
+  // default constructor and doesn't declare one itself, we declare an default
+  // constructor implicitly. This allows us to meanignfully reason about
+  // whether that default constructor is constexpr, trivial, and so on.
+  struct A { constexpr A() {} };
+  struct B : A {
+    using A::A;
+    constexpr B(int) {}
+  };
+  struct C { B b; };
+  constexpr C c;
+
+  struct D { int n; };
+  struct E : D { using D::D; E(int); };
+  static_assert(E().n == 0, "");
+  static_assert(E{}.n == 0, "");
+
+  struct F { E e; };
+  static_assert(F().e.n == 0, "");
+  static_assert(F{}.e.n == 0, "");
+
+  union U { E e; };
+  U u; // OK, trivial default constructor
+
+  struct G { G(); };
+  struct H : D { using D::D; H(int); G g; };
+  union V { H h; }; // expected-note {{field 'h' has a non-trivial default constructor}}
+  V v; // expected-error {{deleted}}
+}
diff --git a/clang/test/CXX/special/class.inhctor/p1.cpp b/clang/test/CXX/special/class.inhctor/p1.cpp
index 389f365b..d116a3a 100644
--- a/clang/test/CXX/special/class.inhctor/p1.cpp
+++ b/clang/test/CXX/special/class.inhctor/p1.cpp
@@ -4,9 +4,9 @@
 // for the wording that used to be there.
 
 struct A {
-  A(...); // expected-note {{candidate constructor}} expected-note {{candidate inherited constructor}}
-  A(int = 0, int = 0, int = 0, int = 0, ...); // expected-note 3{{candidate constructor}} expected-note 3{{candidate inherited constructor}}
-  A(int = 0, int = 0, ...); // expected-note 3{{candidate constructor}} expected-note 3{{candidate inherited constructor}}
+  A(...); // expected-note {{candidate constructor}}
+  A(int = 0, int = 0, int = 0, int = 0, ...); // expected-note 3{{candidate constructor}} expected-note 2{{candidate inherited constructor}}
+  A(int = 0, int = 0, ...); // expected-note 3{{candidate constructor}} expected-note 2{{candidate inherited constructor}}
 
   template<typename T> A(T, int = 0, ...);
 
@@ -14,15 +14,15 @@ struct A {
   template<typename T, int N> A(const T (&)[N], int = 0); // expected-note {{candidate constructor}} expected-note {{candidate inherited constructor}}
 };
 
-struct B : A {
-  using A::A; // expected-note 9{{inherited here}}
+struct B : A { // expected-note {{because base class 'A' has multiple default constructors}}
+  using A::A; // expected-note 6{{inherited here}}
   B(void*);
 };
 
 struct C {} c;
 
 A a0{}; // expected-error {{ambiguous}}
-B b0{}; // expected-error {{ambiguous}}
+B b0{}; // expected-error {{deleted}}
 
 A a1{1}; // expected-error {{ambiguous}}
 B b1{1}; // expected-error {{ambiguous}}
diff --git a/clang/test/CXX/special/class.inhctor/p2.cpp b/clang/test/CXX/special/class.inhctor/p2.cpp
index f84dc64..9bb3f0c 100644
--- a/clang/test/CXX/special/class.inhctor/p2.cpp
+++ b/clang/test/CXX/special/class.inhctor/p2.cpp
@@ -96,7 +96,7 @@ static_assert(ce.k2 == 'x', "");
 
 
 struct TemplateCtors { // expected-note 2{{candidate constructor (the implicit}}
-  constexpr TemplateCtors() {} // expected-note {{candidate inherited constructor}}
+  constexpr TemplateCtors() {}
   template<template<int> class T> TemplateCtors(X<0>, T<0>); // expected-note {{here}} expected-note {{candidate inherited constructor}}
   template<int N> TemplateCtors(X<1>, X<N>); // expected-note {{here}} expected-note {{candidate inherited constructor}}
   template<typename T> TemplateCtors(X<2>, T); // expected-note {{here}} expected-note {{candidate inherited constructor}}
@@ -104,8 +104,8 @@ struct TemplateCtors { // expected-note 2{{candidate constructor (the implicit}}
   template<typename T = int> TemplateCtors(int, int = 0, int = 0);
 };
 
-struct UsingTemplateCtors : TemplateCtors { // expected-note 2{{candidate constructor (the implicit}}
-  using TemplateCtors::TemplateCtors; // expected-note 6{{inherited here}}
+struct UsingTemplateCtors : TemplateCtors { // expected-note 3{{candidate constructor (the implicit}}
+  using TemplateCtors::TemplateCtors; // expected-note 5{{inherited here}}
 
   constexpr UsingTemplateCtors(X<0>, X<0>) {} // expected-note {{not viable}}
   constexpr UsingTemplateCtors(X<1>, X<1>) {} // expected-note {{not viable}}