Skip to content

Commit 1b5404a

Browse files
committed
PR44540: Prefer an inherited default constructor over an initializer
list constructor when initializing from {}. We would previously pick between calling an initializer list constructor and calling a default constructor unstably in this situation, depending on whether the inherited default constructor had already been used elsewhere in the program.
1 parent c6e6988 commit 1b5404a

File tree

3 files changed

+59
-39
lines changed

3 files changed

+59
-39
lines changed

clang/lib/AST/DeclCXX.cpp

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -738,49 +738,55 @@ void CXXRecordDecl::addedMember(Decl *D) {
738738

739739
// Handle constructors.
740740
if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(D)) {
741-
if (!Constructor->isImplicit()) {
742-
// Note that we have a user-declared constructor.
743-
data().UserDeclaredConstructor = true;
741+
if (Constructor->isInheritingConstructor()) {
742+
// Ignore constructor shadow declarations. They are lazily created and
743+
// so shouldn't affect any properties of the class.
744+
} else {
745+
if (!Constructor->isImplicit()) {
746+
// Note that we have a user-declared constructor.
747+
data().UserDeclaredConstructor = true;
748+
749+
// C++ [class]p4:
750+
// A POD-struct is an aggregate class [...]
751+
// Since the POD bit is meant to be C++03 POD-ness, clear it even if
752+
// the type is technically an aggregate in C++0x since it wouldn't be
753+
// in 03.
754+
data().PlainOldData = false;
755+
}
744756

745-
// C++ [class]p4:
746-
// A POD-struct is an aggregate class [...]
747-
// Since the POD bit is meant to be C++03 POD-ness, clear it even if the
748-
// type is technically an aggregate in C++0x since it wouldn't be in 03.
749-
data().PlainOldData = false;
750-
}
757+
if (Constructor->isDefaultConstructor()) {
758+
SMKind |= SMF_DefaultConstructor;
751759

752-
if (Constructor->isDefaultConstructor()) {
753-
SMKind |= SMF_DefaultConstructor;
760+
if (Constructor->isUserProvided())
761+
data().UserProvidedDefaultConstructor = true;
762+
if (Constructor->isConstexpr())
763+
data().HasConstexprDefaultConstructor = true;
764+
if (Constructor->isDefaulted())
765+
data().HasDefaultedDefaultConstructor = true;
766+
}
754767

755-
if (Constructor->isUserProvided())
756-
data().UserProvidedDefaultConstructor = true;
757-
if (Constructor->isConstexpr())
758-
data().HasConstexprDefaultConstructor = true;
759-
if (Constructor->isDefaulted())
760-
data().HasDefaultedDefaultConstructor = true;
761-
}
768+
if (!FunTmpl) {
769+
unsigned Quals;
770+
if (Constructor->isCopyConstructor(Quals)) {
771+
SMKind |= SMF_CopyConstructor;
762772

763-
if (!FunTmpl) {
764-
unsigned Quals;
765-
if (Constructor->isCopyConstructor(Quals)) {
766-
SMKind |= SMF_CopyConstructor;
773+
if (Quals & Qualifiers::Const)
774+
data().HasDeclaredCopyConstructorWithConstParam = true;
775+
} else if (Constructor->isMoveConstructor())
776+
SMKind |= SMF_MoveConstructor;
777+
}
767778

768-
if (Quals & Qualifiers::Const)
769-
data().HasDeclaredCopyConstructorWithConstParam = true;
770-
} else if (Constructor->isMoveConstructor())
771-
SMKind |= SMF_MoveConstructor;
779+
// C++11 [dcl.init.aggr]p1: DR1518
780+
// An aggregate is an array or a class with no user-provided [or]
781+
// explicit [...] constructors
782+
// C++20 [dcl.init.aggr]p1:
783+
// An aggregate is an array or a class with no user-declared [...]
784+
// constructors
785+
if (getASTContext().getLangOpts().CPlusPlus2a
786+
? !Constructor->isImplicit()
787+
: (Constructor->isUserProvided() || Constructor->isExplicit()))
788+
data().Aggregate = false;
772789
}
773-
774-
// C++11 [dcl.init.aggr]p1: DR1518
775-
// An aggregate is an array or a class with no user-provided [or]
776-
// explicit [...] constructors
777-
// C++20 [dcl.init.aggr]p1:
778-
// An aggregate is an array or a class with no user-declared [...]
779-
// constructors
780-
if (getASTContext().getLangOpts().CPlusPlus2a
781-
? !Constructor->isImplicit()
782-
: (Constructor->isUserProvided() || Constructor->isExplicit()))
783-
data().Aggregate = false;
784790
}
785791

786792
// Handle constructors, including those inherited from base classes.

clang/lib/Sema/SemaInit.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4059,7 +4059,7 @@ static void TryConstructorInitialization(Sema &S,
40594059

40604060
// If the initializer list has no elements and T has a default constructor,
40614061
// the first phase is omitted.
4062-
if (!(UnwrappedArgs.empty() && DestRecordDecl->hasDefaultConstructor()))
4062+
if (!(UnwrappedArgs.empty() && S.LookupDefaultConstructor(DestRecordDecl)))
40634063
Result = ResolveConstructorOverload(S, Kind.getLocation(), Args,
40644064
CandidateSet, DestType, Ctors, Best,
40654065
CopyInitialization, AllowExplicit,
@@ -4343,7 +4343,7 @@ static void TryListInitialization(Sema &S,
43434343
// value-initialized.
43444344
if (InitList->getNumInits() == 0) {
43454345
CXXRecordDecl *RD = DestType->getAsCXXRecordDecl();
4346-
if (RD->hasDefaultConstructor()) {
4346+
if (S.LookupDefaultConstructor(RD)) {
43474347
TryValueInitialization(S, Entity, Kind, Sequence, InitList);
43484348
return;
43494349
}

clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p3.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ namespace bullet6 {
8686
const int& i1 = { 1 };
8787
const int& i2 = { 1.1 }; // expected-error {{type 'double' cannot be narrowed to 'int' in initializer list}} expected-note {{silence}} expected-warning {{implicit conversion}}
8888
const int (&iar)[2] = { 1, 2 };
89+
90+
// We interpret "class type with a default constructor" as including the case
91+
// where a default constructor is inherited.
92+
struct X {
93+
X();
94+
X(std::initializer_list<int>) = delete;
95+
};
96+
struct Y : X {
97+
using X::X;
98+
Y(int);
99+
};
100+
Y y1{};
101+
void use() { Y y; }
102+
Y y2{};
89103
}
90104

91105
namespace bullet7 {

0 commit comments

Comments
 (0)