diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 386c7ab4c086ef0de034550e06845aee71f6bc0a..725afc04409aa06f163a0ea880a8e57d36857369 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -3438,6 +3438,23 @@ static bool TryInitializerListConstruction(Sema &S, return true; } +/// Determine if the constructor has the signature of a copy or move +/// constructor for the type T of the class in which it was found. That is, +/// determine if its first parameter is of type T or reference to (possibly +/// cv-qualified) T. +static bool hasCopyOrMoveCtorParam(ASTContext &Ctx, + const ConstructorInfo &Info) { + if (Info.Constructor->getNumParams() == 0) + return false; + + QualType ParmT = + Info.Constructor->getParamDecl(0)->getType().getNonReferenceType(); + QualType ClassT = + Ctx.getRecordType(cast<CXXRecordDecl>(Info.FoundDecl->getDeclContext())); + + return Ctx.hasSameUnqualifiedType(ParmT, ClassT); +} + static OverloadingResult ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc, MultiExprArg Args, @@ -3445,59 +3462,56 @@ ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc, DeclContext::lookup_result Ctors, OverloadCandidateSet::iterator &Best, bool CopyInitializing, bool AllowExplicit, - bool OnlyListConstructors, bool IsListInit) { + bool OnlyListConstructors, bool IsListInit, + bool SecondStepOfCopyInit = false) { CandidateSet.clear(); for (NamedDecl *D : Ctors) { auto Info = getConstructorInfo(D); - if (!Info.Constructor) + if (!Info.Constructor || Info.Constructor->isInvalidDecl()) continue; - bool SuppressUserConversions = false; - - if (!Info.ConstructorTmpl) { - // C++11 [over.best.ics]p4: - // ... and the constructor or user-defined conversion function is a - // candidate by - // - 13.3.1.3, when the argument is the temporary in the second step - // of a class copy-initialization, or - // - 13.3.1.4, 13.3.1.5, or 13.3.1.6 (in all cases), - // user-defined conversion sequences are not considered. - // FIXME: This breaks backward compatibility, e.g. PR12117. As a - // temporary fix, let's re-instate the third bullet above until - // there is a resolution in the standard, i.e., - // - 13.3.1.7 when the initializer list has exactly one element that is - // itself an initializer list and a conversion to some class X or - // reference to (possibly cv-qualified) X is considered for the first - // parameter of a constructor of X. - if ((CopyInitializing || - (IsListInit && Args.size() == 1 && isa<InitListExpr>(Args[0]))) && - Info.Constructor->isCopyOrMoveConstructor()) - SuppressUserConversions = true; - } + if (!AllowExplicit && Info.Constructor->isExplicit()) + continue; - if (!Info.Constructor->isInvalidDecl() && - (AllowExplicit || !Info.Constructor->isExplicit()) && - (!OnlyListConstructors || S.isInitListConstructor(Info.Constructor))) { - if (Info.ConstructorTmpl) - S.AddTemplateOverloadCandidate(Info.ConstructorTmpl, Info.FoundDecl, - /*ExplicitArgs*/ nullptr, Args, - CandidateSet, SuppressUserConversions); - else { - // C++ [over.match.copy]p1: - // - When initializing a temporary to be bound to the first parameter - // of a constructor that takes a reference to possibly cv-qualified - // T as its first argument, called with a single argument in the - // context of direct-initialization, explicit conversion functions - // are also considered. - bool AllowExplicitConv = AllowExplicit && !CopyInitializing && - Args.size() == 1 && - Info.Constructor->isCopyOrMoveConstructor(); - S.AddOverloadCandidate(Info.Constructor, Info.FoundDecl, Args, - CandidateSet, SuppressUserConversions, - /*PartialOverloading=*/false, - /*AllowExplicit=*/AllowExplicitConv); - } + if (OnlyListConstructors && !S.isInitListConstructor(Info.Constructor)) + continue; + + // C++11 [over.best.ics]p4: + // ... and the constructor or user-defined conversion function is a + // candidate by + // - 13.3.1.3, when the argument is the temporary in the second step + // of a class copy-initialization, or + // - 13.3.1.4, 13.3.1.5, or 13.3.1.6 (in all cases), [not handled here] + // - the second phase of 13.3.1.7 when the initializer list has exactly + // one element that is itself an initializer list, and the target is + // the first parameter of a constructor of class X, and the conversion + // is to X or reference to (possibly cv-qualified X), + // user-defined conversion sequences are not considered. + bool SuppressUserConversions = + SecondStepOfCopyInit || + (IsListInit && Args.size() == 1 && isa<InitListExpr>(Args[0]) && + hasCopyOrMoveCtorParam(S.Context, Info)); + + if (Info.ConstructorTmpl) + S.AddTemplateOverloadCandidate(Info.ConstructorTmpl, Info.FoundDecl, + /*ExplicitArgs*/ nullptr, Args, + CandidateSet, SuppressUserConversions); + else { + // C++ [over.match.copy]p1: + // - When initializing a temporary to be bound to the first parameter + // of a constructor [for type T] that takes a reference to possibly + // cv-qualified T as its first argument, called with a single + // argument in the context of direct-initialization, explicit + // conversion functions are also considered. + // FIXME: What if a constructor template instantiates to such a signature? + bool AllowExplicitConv = AllowExplicit && !CopyInitializing && + Args.size() == 1 && + hasCopyOrMoveCtorParam(S.Context, Info); + S.AddOverloadCandidate(Info.Constructor, Info.FoundDecl, Args, + CandidateSet, SuppressUserConversions, + /*PartialOverloading=*/false, + /*AllowExplicit=*/AllowExplicitConv); } } @@ -5348,50 +5362,6 @@ static bool shouldDestroyTemporary(const InitializedEntity &Entity) { llvm_unreachable("missed an InitializedEntity kind?"); } -/// \brief Look for copy and move constructors and constructor templates, for -/// copying an object via direct-initialization (per C++11 [dcl.init]p16). -static void LookupCopyAndMoveConstructors(Sema &S, - OverloadCandidateSet &CandidateSet, - CXXRecordDecl *Class, - Expr *CurInitExpr) { - DeclContext::lookup_result R = S.LookupConstructors(Class); - // The container holding the constructors can under certain conditions - // be changed while iterating (e.g. because of deserialization). - // To be safe we copy the lookup results to a new container. - SmallVector<NamedDecl*, 16> Ctors(R.begin(), R.end()); - for (SmallVectorImpl<NamedDecl *>::iterator - CI = Ctors.begin(), CE = Ctors.end(); CI != CE; ++CI) { - NamedDecl *D = *CI; - auto Info = getConstructorInfo(D); - if (!Info.Constructor) - continue; - - if (!Info.ConstructorTmpl) { - // Handle copy/move constructors, only. - if (Info.Constructor->isInvalidDecl() || - !Info.Constructor->isCopyOrMoveConstructor() || - !Info.Constructor->isConvertingConstructor(/*AllowExplicit=*/true)) - continue; - - S.AddOverloadCandidate(Info.Constructor, Info.FoundDecl, - CurInitExpr, CandidateSet); - continue; - } - - // Handle constructor templates. - if (Info.ConstructorTmpl->isInvalidDecl()) - continue; - - if (!Info.Constructor->isConvertingConstructor(/*AllowExplicit=*/true)) - continue; - - // FIXME: Do we need to limit this to copy-constructor-like - // candidates? - S.AddTemplateOverloadCandidate(Info.ConstructorTmpl, Info.FoundDecl, - nullptr, CurInitExpr, CandidateSet, true); - } -} - /// \brief Get the location at which initialization diagnostics should appear. static SourceLocation getInitializationLoc(const InitializedEntity &Entity, Expr *Initializer) { @@ -5462,39 +5432,24 @@ static ExprResult CopyObject(Sema &S, if (!Class) return CurInit; - // C++0x [class.copy]p32: - // When certain criteria are met, an implementation is allowed to - // omit the copy/move construction of a class object, even if the - // copy/move constructor and/or destructor for the object have - // side effects. [...] - // - when a temporary class object that has not been bound to a - // reference (12.2) would be copied/moved to a class object - // with the same cv-unqualified type, the copy/move operation - // can be omitted by constructing the temporary object - // directly into the target of the omitted copy/move - // - // Note that the other three bullets are handled elsewhere. Copy - // elision for return statements and throw expressions are handled as part - // of constructor initialization, while copy elision for exception handlers - // is handled by the run-time. - bool Elidable = CurInitExpr->isTemporaryObject(S.Context, Class); SourceLocation Loc = getInitializationLoc(Entity, CurInit.get()); // Make sure that the type we are copying is complete. if (S.RequireCompleteType(Loc, T, diag::err_temp_copy_incomplete)) return CurInit; - // Perform overload resolution using the class's copy/move constructors. - // Only consider constructors and constructor templates. Per - // C++0x [dcl.init]p16, second bullet to class types, this initialization + // Perform overload resolution using the class's constructors. Per + // C++11 [dcl.init]p16, second bullet for class types, this initialization // is direct-initialization. OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal); - LookupCopyAndMoveConstructors(S, CandidateSet, Class, CurInitExpr); - - bool HadMultipleCandidates = (CandidateSet.size() > 1); + DeclContext::lookup_result Ctors = S.LookupConstructors(Class); OverloadCandidateSet::iterator Best; - switch (CandidateSet.BestViableFunction(S, Loc, Best)) { + switch (ResolveConstructorOverload( + S, Loc, CurInitExpr, CandidateSet, Ctors, Best, + /*CopyInitializing=*/false, /*AllowExplicit=*/true, + /*OnlyListConstructors=*/false, /*IsListInit=*/false, + /*SecondStepOfCopyInit=*/true)) { case OR_Success: break; @@ -5524,6 +5479,8 @@ static ExprResult CopyObject(Sema &S, return ExprError(); } + bool HadMultipleCandidates = CandidateSet.size() > 1; + CXXConstructorDecl *Constructor = cast<CXXConstructorDecl>(Best->Function); SmallVector<Expr*, 8> ConstructorArgs; CurInit.get(); // Ownership transferred into MultiExprArg, below. @@ -5563,6 +5520,31 @@ static ExprResult CopyObject(Sema &S, if (S.CompleteConstructorCall(Constructor, CurInitExpr, Loc, ConstructorArgs)) return ExprError(); + // C++0x [class.copy]p32: + // When certain criteria are met, an implementation is allowed to + // omit the copy/move construction of a class object, even if the + // copy/move constructor and/or destructor for the object have + // side effects. [...] + // - when a temporary class object that has not been bound to a + // reference (12.2) would be copied/moved to a class object + // with the same cv-unqualified type, the copy/move operation + // can be omitted by constructing the temporary object + // directly into the target of the omitted copy/move + // + // Note that the other three bullets are handled elsewhere. Copy + // elision for return statements and throw expressions are handled as part + // of constructor initialization, while copy elision for exception handlers + // is handled by the run-time. + // + // FIXME: If the function parameter is not the same type as the temporary, we + // should still be able to elide the copy, but we don't have a way to + // represent in the AST how much should be elided in this case. + bool Elidable = + CurInitExpr->isTemporaryObject(S.Context, Class) && + S.Context.hasSameUnqualifiedType( + Best->Function->getParamDecl(0)->getType().getNonReferenceType(), + CurInitExpr->getType()); + // Actually perform the constructor call. CurInit = S.BuildCXXConstructExpr(Loc, T, Best->FoundDecl, Constructor, Elidable, @@ -5598,12 +5580,16 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S, // Find constructors which would have been considered. OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal); - LookupCopyAndMoveConstructors( - S, CandidateSet, cast<CXXRecordDecl>(Record->getDecl()), CurInitExpr); + DeclContext::lookup_result Ctors = + S.LookupConstructors(cast<CXXRecordDecl>(Record->getDecl())); // Perform overload resolution. OverloadCandidateSet::iterator Best; - OverloadingResult OR = CandidateSet.BestViableFunction(S, Loc, Best); + OverloadingResult OR = ResolveConstructorOverload( + S, Loc, CurInitExpr, CandidateSet, Ctors, Best, + /*CopyInitializing=*/false, /*AllowExplicit=*/true, + /*OnlyListConstructors=*/false, /*IsListInit=*/false, + /*SecondStepOfCopyInit=*/true); PartialDiagnostic Diag = S.PDiag(diag::warn_cxx98_compat_temp_copy) << OR << (int)Entity.getKind() << CurInitExpr->getType() @@ -5723,9 +5709,10 @@ PerformConstructorInitialization(Sema &S, // T as its first argument, called with a single argument in the // context of direct-initialization, explicit conversion functions // are also considered. - bool AllowExplicitConv = Kind.AllowExplicit() && !Kind.isCopyInit() && - Args.size() == 1 && - Constructor->isCopyOrMoveConstructor(); + bool AllowExplicitConv = + Kind.AllowExplicit() && !Kind.isCopyInit() && Args.size() == 1 && + hasCopyOrMoveCtorParam(S.Context, + getConstructorInfo(Step.Function.FoundDecl)); // Determine the arguments required to actually perform the constructor // call. diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp index 494cfb57670e71780ce46cdfcb1b72c13cd10e74..7a5caef36e737805006378a965b9ed020b79c9e2 100644 --- a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp +++ b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp @@ -19,7 +19,7 @@ private: }; struct X3 { - X3(); + X3(); // expected-note{{requires 0 arguments, but 1 was provided}} private: X3(X3&); // expected-note{{candidate constructor not viable: expects an l-value for 1st argument}} @@ -42,8 +42,8 @@ struct X4 { // Check for "dangerous" default arguments that could cause recursion. struct X5 { - X5(); - X5(const X5&, const X5& = X5()); // expected-warning{{no viable constructor copying parameter of type 'X5'}} + X5(); // expected-note {{requires 0 arguments}} + X5(const X5&, const X5& = X5()); // expected-warning{{no viable constructor copying parameter of type 'X5'}} expected-note {{requires 2 arguments}} }; void g1(const X1&); diff --git a/test/CXX/drs/dr0xx.cpp b/test/CXX/drs/dr0xx.cpp index 3bb6701b32e1a1d8c007e6df64a262d515144bb4..69cb776c33042b644431af643f6b95dd45a1764e 100644 --- a/test/CXX/drs/dr0xx.cpp +++ b/test/CXX/drs/dr0xx.cpp @@ -643,8 +643,8 @@ namespace dr58 { // dr58: yes namespace dr59 { // dr59: yes template<typename T> struct convert_to { operator T() const; }; - struct A {}; // expected-note 2{{volatile qualifier}} - struct B : A {}; // expected-note 2{{volatile qualifier}} + struct A {}; // expected-note 2{{volatile qualifier}} expected-note 2{{requires 0 arguments}} + struct B : A {}; // expected-note 2{{volatile qualifier}} expected-note 2{{requires 0 arguments}} #if __cplusplus >= 201103L // move constructors // expected-note@-3 2{{volatile qualifier}} // expected-note@-3 2{{volatile qualifier}} @@ -902,7 +902,7 @@ namespace dr84 { // dr84: yes struct C {}; struct B { B(B&); // expected-note {{candidate}} - B(C); + B(C); // expected-note {{no known conversion from 'dr84::B' to 'dr84::C'}} operator C() const; }; A a; diff --git a/test/CXX/drs/dr1xx.cpp b/test/CXX/drs/dr1xx.cpp index 8d368a5a54e8546edd6ad27cb0c3594c4a05e2f1..60d6ccaa875644889535934b7f527110e46f5e4c 100644 --- a/test/CXX/drs/dr1xx.cpp +++ b/test/CXX/drs/dr1xx.cpp @@ -827,7 +827,7 @@ namespace dr177 { // dr177: yes struct B {}; struct A { A(A &); // expected-note {{not viable: expects an l-value}} - A(const B &); + A(const B &); // expected-note {{not viable: no known conversion from 'dr177::A' to}} }; B b; A a = b; // expected-error {{no viable constructor copying variable}} diff --git a/test/CodeGenCXX/copy-constructor-elim-2.cpp b/test/CodeGenCXX/copy-constructor-elim-2.cpp index c263b7ebf95245846a07d5bc1a2e3125fb5aede9..26b6b4851f5a5af65a8503a9c543394117efcc95 100644 --- a/test/CodeGenCXX/copy-constructor-elim-2.cpp +++ b/test/CodeGenCXX/copy-constructor-elim-2.cpp @@ -74,3 +74,23 @@ namespace PR12139 { return a.value; } } + +namespace ElidableCallIsNotCopyCtor { + struct A { A(const A&); }; + struct B : A { + B(B&); + B(A); + B(int); + }; + void f() { + // Here, we construct via B(int) then B(A). The B(A) construction is + // elidable, but we don't have an AST representation for the case where we + // must elide not only a constructor call but also some argument + // conversions, so we don't elide it. + // CHECK-LABEL: define void @_ZN25ElidableCallIsNotCopyCtor1fEv( + // CHECK: call {{.*}} @_ZN25ElidableCallIsNotCopyCtor1BC1Ei( + // CHECK: call {{.*}} @_ZN25ElidableCallIsNotCopyCtor1AC1ERKS0_( + // CHECK: call {{.*}} @_ZN25ElidableCallIsNotCopyCtor1BC1ENS_1AE( + B b = 0; + } +} diff --git a/test/SemaCXX/conditional-expr.cpp b/test/SemaCXX/conditional-expr.cpp index c12efc0be7ef87620d7bff5aeb84aaf60c9326ed..5025990cfd364c809c74d51ca40d8db03c119c3a 100644 --- a/test/SemaCXX/conditional-expr.cpp +++ b/test/SemaCXX/conditional-expr.cpp @@ -262,7 +262,7 @@ namespace PR6757 { struct Foo2 { }; struct Foo3 { - Foo3(); + Foo3(); // expected-note{{requires 0 arguments}} Foo3(Foo3&); // expected-note{{would lose const qualifier}} }; diff --git a/test/SemaCXX/copy-initialization.cpp b/test/SemaCXX/copy-initialization.cpp index d219ee508f035ad70e34767d5fed10694f9597c1..4f6c65cf550c80c32b4212245544aa17e8a5a820 100644 --- a/test/SemaCXX/copy-initialization.cpp +++ b/test/SemaCXX/copy-initialization.cpp @@ -30,7 +30,7 @@ void test(const foo *P) { P->bar(); } // expected-error{{'bar' not viable: 'this namespace PR6757 { struct Foo { - Foo(); + Foo(); // expected-note{{not viable}} Foo(Foo&); // expected-note{{candidate constructor not viable}} }; @@ -71,3 +71,12 @@ namespace DR5 { const S b = 0; } } + +struct A {}; +struct B : A { + B(); + B(B&); + B(A); + B(int); +}; +B b = 0; // ok, calls B(int) then A(const A&) then B(A). diff --git a/test/SemaCXX/cxx0x-initializer-constructor.cpp b/test/SemaCXX/cxx0x-initializer-constructor.cpp index 6202bf620fe3a7c3e88015e4b5d87dd88fac43b5..c10bee917ac03e161c9984f3b82a16e768cd657a 100644 --- a/test/SemaCXX/cxx0x-initializer-constructor.cpp +++ b/test/SemaCXX/cxx0x-initializer-constructor.cpp @@ -267,8 +267,11 @@ namespace PR12120 { struct A { explicit A(int); A(float); }; // expected-note {{declared here}} A a = { 0 }; // expected-error {{constructor is explicit}} - struct B { explicit B(short); B(long); }; // expected-note 2 {{candidate}} + struct B { explicit B(short); B(long); }; // expected-note 4{{candidate}} B b = { 0 }; // expected-error {{ambiguous}} + + struct C { explicit C(short); C(long); }; // expected-note 2{{candidate}} + C c = {{ 0 }}; // expected-error {{ambiguous}} } namespace PR12498 { diff --git a/test/SemaCXX/cxx11-inheriting-ctors.cpp b/test/SemaCXX/cxx11-inheriting-ctors.cpp index 9c33ac05cc5f52b865321851e8cc771451a21858..5ce8d1aa3e0bb783e29526bacadeac284a8adad8 100644 --- a/test/SemaCXX/cxx11-inheriting-ctors.cpp +++ b/test/SemaCXX/cxx11-inheriting-ctors.cpp @@ -1,7 +1,5 @@ // RUN: %clang_cc1 -std=c++11 %s -verify -// expected-no-diagnostics - namespace PR15757 { struct S { }; @@ -56,3 +54,33 @@ namespace InvalidConstruction { template<typename T> int &f(...); int &r = f<C>(0); } + +namespace ExplicitConv { + struct B {}; // expected-note 2{{candidate}} + struct D : B { // expected-note 3{{candidate}} + using B::B; // expected-note 2{{inherited}} + }; + struct X { explicit operator B(); } x; + struct Y { explicit operator D(); } y; + + D dx(x); // expected-error {{no matching constructor}} + D dy(y); +} + +namespace NestedListInit { + struct B { B(); } b; // expected-note 5{{candidate}} + struct D : B { // expected-note 3{{candidate}} + using B::B; // expected-note 2{{inherited}} + }; + // This is a bit weird. We're allowed one pair of braces for overload + // resolution, and one more pair of braces due to [over.ics.list]/2. + B b1 = {b}; + B b2 = {{b}}; + B b3 = {{{b}}}; // expected-error {{no match}} + // This is the same, but we get one call to D's version of B::B(const B&) + // before the two permitted calls to D::D(D&&). + D d1 = {b}; + D d2 = {{b}}; + D d3 = {{{b}}}; + D d4 = {{{{b}}}}; // expected-error {{no match}} +} diff --git a/test/SemaCXX/cxx98-compat-flags.cpp b/test/SemaCXX/cxx98-compat-flags.cpp index f90ad345e975b6e00745e2d6a6c7210cba04f9e0..62d687c49cf793a36274ed08b3d86dc3a1d6a3e5 100644 --- a/test/SemaCXX/cxx98-compat-flags.cpp +++ b/test/SemaCXX/cxx98-compat-flags.cpp @@ -16,7 +16,7 @@ namespace CopyCtorIssues { Private(const Private&); // expected-note {{declared private here}} }; struct NoViable { - NoViable(); + NoViable(); // expected-note {{not viable}} NoViable(NoViable&); // expected-note {{not viable}} }; struct Ambiguous { diff --git a/test/SemaCXX/cxx98-compat-pedantic.cpp b/test/SemaCXX/cxx98-compat-pedantic.cpp index 8b0dcc871327f4a4686191ae53992d87272c0591..c72c44ad5feb68d967969fb36c7933465ab976b7 100644 --- a/test/SemaCXX/cxx98-compat-pedantic.cpp +++ b/test/SemaCXX/cxx98-compat-pedantic.cpp @@ -55,7 +55,7 @@ namespace CopyCtorIssues { Private(const Private&); // expected-note {{declared private here}} }; struct NoViable { - NoViable(); + NoViable(); // expected-note {{not viable}} NoViable(NoViable&); // expected-note {{not viable}} }; struct Ambiguous {