Implement the likely resolution of core issue 253.

[thakis]

Summary

C++11 requires const objects to have a user-provided constructor, even for classes without any fields. DR 253 relaxes this to say "If the implicit default constructor initializes all subobjects, no initializer should be required."

clang is currently the only compiler that implements this C++11 rule, and e.g. libstdc++ relies on something like DR 253 to compile in newer versions. So this patch makes it possible to build code that says `const vector<int> v;' again when using libstdc++5.2 and _GLIBCXX_DEBUG (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60284).

Fixes PR23381.

[thakis]

ping :-)

[rsmith]

Can we implement all of DR253 instead? (That is, don't restrict this change to libstdc++'s types.) We don't have precise wording for DR253 yet, but do have the guidance that "if the implicit default constructor initializes all subobjects, no initializer should be required."

[thakis]

Will do, thanks! In "If the implicit default constructor initializes all subobjects, no initializer should be required", is "initializes all subobjects meant recursively? I.e. is this ok according to that language?

`
struct Inner {
  Inner() = default;
  int i;  // not initialized, but Inner has a default ctor
};

struct S {
  Inner inner;
};

const S s;
`

[rsmith]

In D16552#346678, @thakis wrote:

Will do, thanks! In "If the implicit default constructor initializes all subobjects, no initializer should be required", is "initializes all subobjects meant recursively?

I think so, or more specifically, the implicit initialization would need to be valid if each member were itself const.

struct Inner {
  Inner() = default;
  int i;  // not initialized, but Inner has a default ctor
};

struct S {
  Inner inner;
};

const S s;

This would be ill-formed because S::S() would notionally default-initialize s.inner of type const Inner, which would notionally default-initialize s.inner.i of type const int, which would be ill-formed.

[thakis]

Checkpointing; not production quality. I tried implementing the general DR 253 rule, and this patch mostly does that. It currently walks all fields and all bases for every const record decl; there should probably be a cache for "are all fields obviously initialized"; probably on CXXRecordDecl. I had to change some test cases, but nothing surprising.

However, while this patch does fix const std::unordered_map<int, int> um; with libstdc++5.3, it does _not_ fix std::vector<int> v; or const std::unordered_map<int, int> um; when -D_GLIBCXX_DEBUG=1 is passed in. Investigating that case more now.

[thakis]

Ok, that was just a bug. Now all libstdc++5.3 cases I tried build. Still not production-quality, but the overall direction won't change. If you want to comment on that, please do, else I'll ping this again after I've cleaned it up.

[rsmith]

The direction here looks fine to me.

lib/Sema/SemaInit.cpp

369

I wonder if this recursion through non-inline namespaces is really warranted; can we replace the call to this function with R->isInStdNamespace()?

434–436

(Please commit this separately if you still want to make this change.)

[thakis]

Ok, now cleaned up and caches the "is this ok?" state on a per-record level.

[thakis]

include/clang/AST/DeclCXX.h

410 ↗

(On Diff #48248)

This should be :2 of course.

[rsmith]

include/clang/AST/DeclCXX.h

405–416 ↗

(On Diff #48248)

Is it worth caching this? (If so, the bit-field should occupy at most 2 bits...)

1286 ↗

(On Diff #48248)

Declaring a const what?

lib/AST/DeclCXX.cpp

396–425 ↗

(On Diff #48248)

For all the other fields of this kind, we compute the flag as bases and members are added to the class. It seems like we can do the same here, and all else being equal, it would be better to not deviate from the existing pattern without good reason.

The most natural way to do this would probably be to store a single flag for HasUninitializedFields, and then compute whether default initialization would leave a field uninitialized as !hasUserProvidedDefaultConstructor() && HasUninitializedFields.

403–413 ↗

(On Diff #48248)

I think you should skip mutable members here, as the corresponding subobject of a const object is not itself const.

[thakis]

address comments

[thakis]

thanks!

include/clang/AST/DeclCXX.h

405–416 ↗

(On Diff #48248)

The motivation is that a chain of

struct A0 {};
struct A1 { const A0 a; }
struct A2 { const A1 a; }
struct A3 { const A2 a; }

needs O(n^2) without caching. Changed the bitfield to 2 bits.

1286 ↗

(On Diff #48248)

Done (I think?)

lib/AST/DeclCXX.cpp

396–425 ↗

(On Diff #48248)

My reasoning here was that for the vast majority of types, we'll never have a const variable of that type, so computing this for all types seemed like a waste.

[thakis]

update test

lib/AST/DeclCXX.cpp

403–413 ↗

(On Diff #48373)

I did this, but the test suite informs me that this contradicts the resolution of DR497: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#497 I guess 253 supersedes 497; I'll update the test.

[rsmith]

lib/AST/DeclCXX.cpp

396–425 ↗

(On Diff #48376)

I don't expect this to be a measurable cost either way. I would strongly prefer to use the same pattern for all of these flags unless we have a really good reason not to.

lib/Serialization/ASTReaderDecl.cpp

1517–1562 ↗

(On Diff #48376)

You should update this block too.

test/CXX/drs/dr4xx.cpp

1200 ↗

(On Diff #48376)

Write this as "dr497: sup 253", and rerun www/make_cxx_dr_status to generate a new www/cxx_dr_status.html file. (You'll need to grab a cwg_index.html from the WG21 website.)

[thakis]

Thanks! All done.

[rsmith]

Thanks, LGTM!

If it's convenient, it'd be nice to check in a new cxx_dr_status.html without your change first, as a separate commit, so it's more obvious which parts of the file are changed by this commit and which are changed by the new cwg_index.html.

[thakis]

Thanks! Landed the cxx_dr_status.html update in r261295 and this change in r261297.

lib/Sema/SemaInit.cpp

InitListChecker(Sema &S, const InitializedEntity &Entity,
                   InitListExpr *IL, QualType &T, bool VerifyOnly);
   bool HadError() { return hadError; }
 
   // @brief Retrieves the fully-structured initializer list used for
   // semantic analysis and code generation.
   InitListExpr *getFullyStructuredList() const { return FullyStructuredList; }
 };
 
+bool IsContainedInNamespaceStd(Sema& S, CXXRecordDecl *R) {
+  for (NamespaceDecl *ND = dyn_cast<NamespaceDecl>(R->getDeclContext()); ND;
+       ND = dyn_cast<NamespaceDecl>(ND->getParent())) {
+    if (S.getStdNamespace()->InEnclosingNamespaceSetOf(ND))
+      return true;
+  }
+  return false;
+}
+
 } // end anonymous namespace
 
 ExprResult InitListChecker::PerformEmptyInit(Sema &SemaRef,
                                              SourceLocation Loc,
                                              const InitializedEntity &Entity,
                                              bool VerifyOnly) {
   InitializationKind Kind = InitializationKind::CreateValue(Loc, Loc, Loc,
                                                             true);
if (!InitSeq && EmptyInitList && InitSeq.getFailureKind() ==
     (void)O;
     assert(O == OR_Success && "Inconsistent overload resolution");
     CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
     CXXRecordDecl *R = CtorDecl->getParent();
 
     if (CtorDecl->getMinRequiredArguments() == 0 &&
         CtorDecl->isExplicit() && R->getDeclName() &&
         SemaRef.SourceMgr.isInSystemHeader(CtorDecl->getLocation())) {
-      bool IsInStd = false;
-      for (NamespaceDecl *ND = dyn_cast<NamespaceDecl>(R->getDeclContext());
-           ND && !IsInStd; ND = dyn_cast<NamespaceDecl>(ND->getParent())) {
-        if (SemaRef.getStdNamespace()->InEnclosingNamespaceSetOf(ND))
-          IsInStd = true;
-      }
-
-      if (IsInStd && llvm::StringSwitch<bool>(R->getName()) 
+      if (IsContainedInNamespaceStd(SemaRef, R) &&
+          llvm::StringSwitch<bool>(R->getName())
               .Cases("basic_string", "deque", "forward_list", true)
               .Cases("list", "map", "multimap", "multiset", true)
               .Cases("priority_queue", "queue", "set", "stack", true)
               .Cases("unordered_map", "unordered_set", "vector", true)
               .Default(false)) {
         InitSeq.InitializeFrom(
             SemaRef, Entity,
             InitializationKind::CreateValue(Loc, Loc, Loc, true),
             MultiExprArg(), /*TopLevelOfInitList=*/false);
if (!Constructor->isInvalidDecl() &&
       }
     }
   }
 
   // Perform overload resolution and return the result.
   return CandidateSet.BestViableFunction(S, DeclLoc, Best);
 }
 
+static bool HasUninitializedFields(const CXXRecordDecl* R) {
+  for (const auto *F : R->fields()) {
+    if (F->hasInClassInitializer() || F->isUnnamedBitfield())
+      continue;
+    if (CXXRecordDecl *FieldType = F->getType()->getAsCXXRecordDecl()) {
+      if (FieldType->hasUserProvidedDefaultConstructor())
+        continue;
+      if (HasUninitializedFields(FieldType)) {
+FieldType->dump();
+        return true;
+      } else {
+        // Has class type but class has no user-provided default ctor. Only
+        // a problem if that class has uninitialized fields.
+        if (!HasUninitializedFields(FieldType))
+          continue;
+      }
+    }
+    // field has no intializer
+    F->dump();
+    return true;
+  }
+  for (const auto& BI : R->bases()) {
+    const RecordType *RT = BI.getType()->getAs<RecordType>();
+    CXXRecordDecl *Base = cast<CXXRecordDecl>(RT->getDecl());
+    if (HasUninitializedFields(Base)) {
+Base->dump();
+      return true;
+    }
+  }
+  return false;
+}
+
 /// \brief Attempt initialization by constructor (C++ [dcl.init]), which
 /// enumerates the constructors of the initialized entity and performs overload
 /// resolution to select the best.
 /// \param IsListInit     Is this list-initialization?
 /// \param IsInitListCopy Is this non-list-initialization resulting from a
 ///                       list-initialization from {x} where x is the same
 ///                       type as the entity?
 static void TryConstructorInitialization(Sema &S,
Sequence.SetOverloadFailure(IsListInit ?
                                 Result);
     return;
   }
 
   // C++11 [dcl.init]p6:
   //   If a program calls for the default initialization of an object
   //   of a const-qualified type T, T shall be a class type with a
   //   user-provided default constructor.
+  CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
   if (Kind.getKind() == InitializationKind::IK_Default &&
-      Entity.getType().isConstQualified() &&
-      !cast<CXXConstructorDecl>(Best->Function)->isUserProvided()) {
+      Entity.getType().isConstQualified() && !CtorDecl->isUserProvided()) {
+    // Some libstdc++ types use `MyType() = default;` which technically isn't
+    // a user-provided default constructor.  For types that don't have
+    // uninitialized fields, suppress the error.  See also core issue 253.
+    CXXRecordDecl *R = CtorDecl->getParent();
+    bool AllowConstDefaultInit = !HasUninitializedFields(R);
+    if (!AllowConstDefaultInit) {
       if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
         Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);
       return;
     }
+  }
 
   // C++11 [over.match.list]p1:
   //   In copy-list-initialization, if an explicit constructor is chosen, the
   //   initializer is ill-formed.
-  CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
   if (IsListInit && !Kind.AllowExplicit() && CtorDecl->isExplicit()) {
     Sequence.SetFailed(InitializationSequence::FK_ExplicitConstructor);
     return;
   }
 
   // Add the constructor initialization step. Any cv-qualification conversion is
   // subsumed by the initialization.
   bool HadMultipleCandidates = (CandidateSet.size() > 1);

test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp

 static_assert(!noexcept(E5(static_cast<E5&&>(e5))), "");
 static_assert(!noexcept(E5(e5)), "");
 static_assert(!noexcept(e5 = E5()), "");
 static_assert(!noexcept(e5 = e5), "");
 
 namespace PR13492 {
   struct B {
     B() = default;
+    int field;
   };
 
   void f() {
     const B b; // expected-error {{default initialization of an object of const type 'const PR13492::B' without a user-provided default constructor}}
   }
 }

test/CXX/dcl.decl/dcl.init/p6.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 // FIXME: Very incomplete!
 
 // If a program calls for the default initialization of an object of a
 // const-qualified type T, T shall be a class type with a
-// user-provided default constructor.
-struct MakeNonPOD { MakeNonPOD(); };
+// user-provided default constructor, except if T has no uninitialized fields.
+struct MakeNonPOD { MakeNonPOD(); int field; };
 struct NoUserDefault : public MakeNonPOD { };
 struct HasUserDefault { HasUserDefault(); };
 
 void test_const_default_init() {
   const NoUserDefault x1; // expected-error{{default initialization of an object of const type 'const NoUserDefault' without a user-provided default constructor}}
   const HasUserDefault x2;
   const int x3; // expected-error{{default initialization of an object of const type 'const int'}}
 }
 
 // rdar://8501008
-struct s0 {};
+struct s0 { int field; };
 struct s1 { static const s0 foo; };
 const struct s0 s1::foo; // expected-error{{default initialization of an object of const type 'const struct s0' without a user-provided default constructor}}
 
 template<typename T>
 struct s2 {
   static const s0 foo;
 };
 
 template<> const struct s0 s2<int>::foo; // okay

test/SemaCXX/attr-selectany.cpp

 
 namespace { class Internal {}; }
 __declspec(selectany) auto x8 = Internal(); // expected-error {{'selectany' can only be applied to data items with external linkage}}
 
 
 // The D3D11 headers do something like this.  MSVC doesn't error on this at
 // all, even without the __declspec(selectany), in violation of the standard.
 // We fall back to a warning for selectany to accept headers.
-struct SomeStruct {};
+struct SomeStruct {
+  int foo;
+};
 extern const __declspec(selectany) SomeStruct some_struct; // expected-warning {{default initialization of an object of const type 'const SomeStruct' without a user-provided default constructor is a Microsoft extension}}
 
 // It should be possible to redeclare variables that were defined
 // __declspec(selectany) previously.
 extern const SomeStruct some_struct;
 
 // Without selectany, this should stay an error.
 const SomeStruct some_struct2; // expected-error {{default initialization of an object of const type 'const SomeStruct' without a user-provided default constructor}}

test/SemaCXX/constexpr-value-init.cpp

 // RUN: %clang_cc1 %s -Wno-uninitialized -std=c++11 -fsyntax-only -verify
 
 struct A {
   constexpr A() : a(b + 1), b(a + 1) {} // expected-note {{outside its lifetime}}
   int a;
   int b;
 };
 struct B {
   A a;
 };
 
 constexpr A a; // ok, zero initialization precedes static initialization
 void f() {
   constexpr A a; // expected-error {{constant expression}} expected-note {{in call to 'A()'}}
 }
 
-constexpr B b1; // expected-error {{without a user-provided default constructor}}
+constexpr B b1; // ok
 constexpr B b2 = B(); // ok
 static_assert(b2.a.a == 1, "");
 static_assert(b2.a.b == 2, "");
 
 struct C {
   int c;
 };
 struct D : C { int d; };

test/SemaCXX/cxx0x-cursory-default-delete.cpp

 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 
 struct non_copiable {
   non_copiable(const non_copiable&) = delete; // expected-note {{marked deleted here}}
   non_copiable& operator = (const non_copiable&) = delete; // expected-note {{explicitly deleted}}
   non_copiable() = default;
 };
 
 struct non_const_copy {
   non_const_copy(non_const_copy&);
   non_const_copy& operator = (non_const_copy&) &;
   non_const_copy& operator = (non_const_copy&) &&;
   non_const_copy() = default; // expected-note {{not viable}}
+  int uninit_field;
 };
 non_const_copy::non_const_copy(non_const_copy&) = default; // expected-note {{not viable}}
 non_const_copy& non_const_copy::operator = (non_const_copy&) & = default; // expected-note {{not viable}}
 non_const_copy& non_const_copy::operator = (non_const_copy&) && = default; // expected-note {{not viable}}
 
 void fn1 () {
   non_copiable nc;
   non_copiable nc2 = nc; // expected-error {{deleted constructor}}
   nc = nc; // expected-error {{deleted operator}}
 
   non_const_copy ncc;
   non_const_copy ncc2 = ncc;
   ncc = ncc2;
   const non_const_copy cncc{};
   const non_const_copy cncc1; // expected-error {{default initialization of an object of const type 'const non_const_copy' without a user-provided default constructor}}
   non_const_copy ncc3 = cncc; // expected-error {{no matching}}
   ncc = cncc; // expected-error {{no viable overloaded}}
 };
 
+struct no_fields { };
+struct all_init {
+  int a = 0;
+  int b = 0;
+};
+struct some_init {
+  int a = 0;
+  int b;
+  int c = 0;
+};
+struct some_init_def {
+  some_init_def() = default;
+  int a = 0;
+  int b;
+  int c = 0;
+};
+struct some_init_ctor {
+  some_init_ctor();
+  int a = 0;
+  int b;
+  int c = 0;
+};
+struct sub_some_init : public some_init_def { };
+struct sub_some_init_ctor : public some_init_def {
+  sub_some_init_ctor();
+};
+struct some_init_container {
+  some_init_def sid;
+};
+struct some_init_container_ctor {
+  some_init_container_ctor();
+  some_init_def sid;
+};
+struct no_fields_container {
+  no_fields nf;
+};
+
+void constobjs() {
+  const no_fields nf; // ok
+  const all_init ai; // ok
+  const some_init si; // expected-error {{default initialization of an object of const type 'const some_init' without a user-provided default constructor}}
+  const some_init_def sid; // expected-error {{default initialization of an object of const type 'const some_init_def' without a user-provided default constructor}}
+  const some_init_ctor sic; // ok
+  const sub_some_init ssi; // expected-error {{default initialization of an object of const type 'const sub_some_init' without a user-provided default constructor}}
+  const sub_some_init_ctor ssic; // ok
+  const some_init_container sicon; // expected-error {{default initialization of an object of const type 'const some_init_container' without a user-provided default constructor}}
+  const some_init_container_ctor siconc; // ok
+  const no_fields_container nfc; // ok
+}
+
 struct non_const_derived : non_const_copy {
   non_const_derived(const non_const_derived&) = default; // expected-error {{requires it to be non-const}}
   non_const_derived& operator =(non_const_derived&) = default;
 };
 
 struct bad_decls {
   bad_decls(volatile bad_decls&) = default; // expected-error {{may not be volatile}}
   bad_decls&& operator = (bad_decls) = default; // expected-error {{lvalue reference}} expected-error {{must return 'bad_decls &'}}

test/SemaCXX/illegal-member-initialization.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s 
 
 struct A {
    A() : value(), cvalue() { } // expected-error {{reference to type 'int' requires an initializer}}
    int &value;
    const int cvalue;
 };
 
 struct B {
+  int field;
 };
 
 struct X {
    X() { }      // expected-error {{constructor for 'X' must explicitly initialize the reference member 'value'}} \
                 // expected-error {{constructor for 'X' must explicitly initialize the const member 'cvalue'}} \
                 // expected-error {{constructor for 'X' must explicitly initialize the reference member 'b'}} \
                 // expected-error {{constructor for 'X' must explicitly initialize the const member 'cb'}}
    int &value; // expected-note{{declared here}}