From 523d46af407f32fc53861e6f068e8076d4fe84a8 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Sun, 18 Apr 2010 07:40:54 +0000 Subject: [PATCH] In C++98/03, when binding a reference to an rvalue of reference-compatible type, the implementation is permitted to make a copy of the rvalue (or many such copies, even). However, even though we don't make that copy, we are required to check for the presence of a suitable copy constructor. With this change, we do. Note that in C++0x we are not allowed to make these copies, so we test both dialects separately. Also note the FIXME in one of the C++03 tests, where we are not instantiating default function arguments for the copy constructor we pick (but do not call). The fix is obvious; eliminating the infinite recursion it causes is not. Will address that next. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101704 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/Sema.h | 2 +- lib/Sema/SemaInit.cpp | 86 ++++++++++++++++--- lib/Sema/SemaInit.h | 23 ++++- .../dcl.init.ref/p16-cxx0x-no-extra-copy.cpp | 51 +++++++++++ .../dcl.init.ref/p5-cxx03-extra-copy.cpp | 51 +++++++++++ 5 files changed, 197 insertions(+), 16 deletions(-) create mode 100644 test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp create mode 100644 test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index ef76882ac0..e057a924c7 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -2154,7 +2154,7 @@ public: bool CompleteConstructorCall(CXXConstructorDecl *Constructor, MultiExprArg ArgsPtr, - SourceLocation Loc, + SourceLocation Loc, ASTOwningVector<&ActionBase::DeleteExpr> &ConvertedArgs); virtual TypeTy *getDestructorName(SourceLocation TildeLoc, diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index db7c1b396a..59f4393e55 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -1987,6 +1987,7 @@ void InitializationSequence::Step::Destroy() { case SK_CastDerivedToBaseLValue: case SK_BindReference: case SK_BindReferenceToTemporary: + case SK_ExtraneousCopyToTemporary: case SK_UserConversion: case SK_QualificationConversionRValue: case SK_QualificationConversionLValue: @@ -2067,6 +2068,13 @@ void InitializationSequence::AddReferenceBindingStep(QualType T, Steps.push_back(S); } +void InitializationSequence::AddExtraneousCopyToTemporary(QualType T) { + Step S; + S.Kind = SK_ExtraneousCopyToTemporary; + S.Type = T; + Steps.push_back(S); +} + void InitializationSequence::AddUserConversionStep(FunctionDecl *Function, DeclAccessPair FoundDecl, QualType T) { @@ -2483,6 +2491,18 @@ static void TryReferenceInitialization(Sema &S, // reference-compatible with "cv2 T2", or if (InitLvalue != Expr::LV_Valid && RefRelationship >= Sema::Ref_Compatible_With_Added_Qualification) { + // The corresponding bullet in C++03 [dcl.init.ref]p5 gives the + // compiler the freedom to perform a copy here or bind to the + // object, while C++0x requires that we bind directly to the + // object. Hence, we always bind to the object without making an + // extra copy. However, in C++03 requires that we check for the + // presence of a suitable copy constructor: + // + // The constructor that would be used to make the copy shall + // be callable whether or not the copy is actually done. + if (!S.getLangOptions().CPlusPlus0x) + Sequence.AddExtraneousCopyToTemporary(cv2T2); + if (DerivedToBase) Sequence.AddDerivedToBaseCastStep( S.Context.getQualifiedType(T1, T2Quals), @@ -3108,15 +3128,35 @@ static bool shouldBindAsTemporary(const InitializedEntity &Entity) { llvm_unreachable("missed an InitializedEntity kind?"); } +/// \brief Make a (potentially elidable) temporary copy of the object +/// provided by the given initializer by calling the appropriate copy +/// constructor. +/// +/// \param S The Sema object used for type-checking. +/// +/// \param T The type of the temporary object, which must either by +/// the type of the initializer expression or a superclass thereof. +/// +/// \param Enter The entity being initialized. +/// +/// \param CurInit The initializer expression. +/// +/// \param IsExtraneousCopy Whether this is an "extraneous" copy that +/// is permitted in C++03 (but not C++0x) when binding a reference to +/// an rvalue. +/// +/// \returns An expression that copies the initializer expression into +/// a temporary object, or an error expression if a copy could not be +/// created. static Sema::OwningExprResult CopyObject(Sema &S, + QualType T, const InitializedEntity &Entity, - const InitializationKind &Kind, - Sema::OwningExprResult CurInit) { + Sema::OwningExprResult CurInit, + bool IsExtraneousCopy) { // Determine which class type we're copying to. Expr *CurInitExpr = (Expr *)CurInit.get(); CXXRecordDecl *Class = 0; - if (const RecordType *Record - = Entity.getType().getNonReferenceType()->getAs()) + if (const RecordType *Record = T->getAs()) Class = cast(Record->getDecl()); if (!Class) return move(CurInit); @@ -3137,7 +3177,7 @@ static Sema::OwningExprResult CopyObject(Sema &S, // not yet) handled as part of constructor initialization, while // copy elision for exception handlers is handled by the run-time. bool Elidable = CurInitExpr->isTemporaryObject() && - S.Context.hasSameUnqualifiedType(Entity.getType(), CurInitExpr->getType()); + S.Context.hasSameUnqualifiedType(T, CurInitExpr->getType()); SourceLocation Loc; switch (Entity.getKind()) { case InitializedEntity::EK_Result: @@ -3217,9 +3257,23 @@ static Sema::OwningExprResult CopyObject(Sema &S, CXXConstructorDecl *Constructor = cast(Best->Function); ASTOwningVector<&ActionBase::DeleteExpr> ConstructorArgs(S); CurInit.release(); // Ownership transferred into MultiExprArg, below. + + S.CheckConstructorAccess(Loc, Constructor, + Best->FoundDecl.getAccess()); + + if (IsExtraneousCopy) { + // If this is a totally extraneous copy for C++03 reference + // binding purposes, just return the original initialization + // expression. + + // FIXME: We'd like to call CompleteConstructorCall below, so that + // we instantiate default arguments and such. + return S.Owned(CurInitExpr); + } // Determine the arguments required to actually perform the - // constructor call (we might have derived-to-base conversions). + // constructor call (we might have derived-to-base conversions, or + // the copy constructor may have default arguments). if (S.CompleteConstructorCall(Constructor, Sema::MultiExprArg(S, (void **)&CurInitExpr, @@ -3227,12 +3281,7 @@ static Sema::OwningExprResult CopyObject(Sema &S, Loc, ConstructorArgs)) return S.ExprError(); - S.CheckConstructorAccess(Loc, Constructor, - Best->FoundDecl.getAccess()); - - return S.BuildCXXConstructExpr(Loc, Entity.getType().getNonReferenceType(), - Constructor, - Elidable, + return S.BuildCXXConstructExpr(Loc, T, Constructor, Elidable, move_arg(ConstructorArgs)); } @@ -3326,6 +3375,7 @@ InitializationSequence::Perform(Sema &S, case SK_CastDerivedToBaseLValue: case SK_BindReference: case SK_BindReferenceToTemporary: + case SK_ExtraneousCopyToTemporary: case SK_UserConversion: case SK_QualificationConversionLValue: case SK_QualificationConversionRValue: @@ -3421,6 +3471,11 @@ InitializationSequence::Perform(Sema &S, break; + case SK_ExtraneousCopyToTemporary: + CurInit = CopyObject(S, Step->Type, Entity, move(CurInit), + /*IsExtraneousCopy=*/true); + break; + case SK_UserConversion: { // We have a user-defined conversion that invokes either a constructor // or a conversion function. @@ -3497,7 +3552,8 @@ InitializationSequence::Perform(Sema &S, IsLvalue)); if (RequiresCopy) - CurInit = CopyObject(S, Entity, Kind, move(CurInit)); + CurInit = CopyObject(S, Entity.getType().getNonReferenceType(), Entity, + move(CurInit), /*IsExtraneousCopy=*/false); break; } @@ -4040,6 +4096,10 @@ void InitializationSequence::dump(llvm::raw_ostream &OS) const { OS << "bind reference to a temporary"; break; + case SK_ExtraneousCopyToTemporary: + OS << "extraneous C++03 copy to temporary"; + break; + case SK_UserConversion: OS << "user-defined conversion via " << S->Function.Function; break; diff --git a/lib/Sema/SemaInit.h b/lib/Sema/SemaInit.h index 65ee42042c..db987ec296 100644 --- a/lib/Sema/SemaInit.h +++ b/lib/Sema/SemaInit.h @@ -416,6 +416,10 @@ public: SK_BindReference, /// \brief Reference binding to a temporary. SK_BindReferenceToTemporary, + /// \brief An optional copy of a temporary object to another + /// temporary object, which is permitted (but not required) by + /// C++98/03 but not C++0x. + SK_ExtraneousCopyToTemporary, /// \brief Perform a user-defined conversion, either via a conversion /// function or via a constructor. SK_UserConversion, @@ -629,11 +633,26 @@ public: /// \brief Add a new step binding a reference to an object. /// - /// \param BindingTemporary true if we are binding a reference to a temporary + /// \param BindingTemporary True if we are binding a reference to a temporary /// object (thereby extending its lifetime); false if we are binding to an /// lvalue or an lvalue treated as an rvalue. + /// + /// \param UnnecessaryCopy True if we should check for a copy + /// constructor for a completely unnecessary but void AddReferenceBindingStep(QualType T, bool BindingTemporary); - + + /// \brief Add a new step that makes an extraneous copy of the input + /// to a temporary of the same class type. + /// + /// This extraneous copy only occurs during reference binding in + /// C++98/03, where we are permitted (but not required) to introduce + /// an extra copy. At a bare minimum, we must check that we could + /// call the copy constructor, and produce a diagnostic if the copy + /// constructor is inaccessible or no copy constructor matches. + // + /// \param T The type of the temporary being created. + void AddExtraneousCopyToTemporary(QualType T); + /// \brief Add a new step invoking a conversion function, which is either /// a constructor or a conversion function. void AddUserConversionStep(FunctionDecl *Function, diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp new file mode 100644 index 0000000000..38bf9f7143 --- /dev/null +++ b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++0x %s + +// C++03 requires that we check for a copy constructor when binding a +// reference to a reference-compatible rvalue, since we are allowed to +// make a copy. C++0x does not permit the copy, so ensure that we +// don't diagnose cases where the copy constructor is unavailable. + +struct X1 { + X1(); + explicit X1(const X1&); +}; + +struct X2 { + X2(); + +private: + X2(const X2&); +}; + +struct X3 { + X3(); + +private: + X3(X3&); +}; + +template +T get_value_badly() { + double *dp = 0; + T *tp = dp; // FIXME: Should get an error here, from instantiating the + // default argument of X4 + return T(); +} + +template +struct X4 { + X4(); + X4(const X4&, T = get_value_badly()); +}; + +void g1(const X1&); +void g2(const X2&); +void g3(const X3&); +void g4(const X4&); + +void test() { + g1(X1()); + g2(X2()); + g3(X3()); + g4(X4()); +} 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 new file mode 100644 index 0000000000..5dd6861357 --- /dev/null +++ b/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// C++03 requires that we check for a copy constructor when binding a +// reference to a temporary, since we are allowed to make a copy, Even +// though we don't actually make that copy, make sure that we diagnose +// cases where that copy constructor is somehow unavailable. + +struct X1 { + X1(); + explicit X1(const X1&); +}; + +struct X2 { + X2(); + +private: + X2(const X2&); // expected-note{{declared private here}} +}; + +struct X3 { + X3(); + +private: + X3(X3&); // expected-note{{candidate constructor not viable: no known conversion from 'X3' to 'X3 &' for 1st argument}} +}; + +template +T get_value_badly() { + double *dp = 0; + T *tp = dp; // FIXME: Should get an error here, from instantiating the + // default argument of X4 + return T(); +} + +template +struct X4 { + X4(); + X4(const X4&, T = get_value_badly()); +}; + +void g1(const X1&); +void g2(const X2&); +void g3(const X3&); +void g4(const X4&); + +void test() { + g1(X1()); // expected-error{{no viable constructor copying parameter of type 'X1'}} + g2(X2()); // expected-error{{calling a private constructor of class 'X2'}} + g3(X3()); // expected-error{{no viable constructor copying parameter of type 'X3'}} + g4(X4()); +} -- 2.40.0