[cfe-commits] r101704 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaInit.cpp lib/Sema/SemaInit.h test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp

Douglas Gregor dgregor at apple.com
Sun Apr 18 00:40:54 PDT 2010


Author: dgregor
Date: Sun Apr 18 02:40:54 2010
New Revision: 101704

URL: http://llvm.org/viewvc/llvm-project?rev=101704&view=rev
Log:
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.


Added:
    cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp   (with props)
    cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp   (with props)
Modified:
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaInit.cpp
    cfe/trunk/lib/Sema/SemaInit.h

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=101704&r1=101703&r2=101704&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Sun Apr 18 02:40:54 2010
@@ -2154,7 +2154,7 @@
 
   bool CompleteConstructorCall(CXXConstructorDecl *Constructor,
                                MultiExprArg ArgsPtr,
-                               SourceLocation Loc,                                    
+                               SourceLocation Loc,
                       ASTOwningVector<&ActionBase::DeleteExpr> &ConvertedArgs);
     
   virtual TypeTy *getDestructorName(SourceLocation TildeLoc,

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=101704&r1=101703&r2=101704&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Apr 18 02:40:54 2010
@@ -1987,6 +1987,7 @@
   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 @@
   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 @@
     //         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 @@
   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<RecordType>())
+  if (const RecordType *Record = T->getAs<RecordType>())
     Class = cast<CXXRecordDecl>(Record->getDecl());
   if (!Class)
     return move(CurInit);
@@ -3137,7 +3177,7 @@
   // 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 @@
   CXXConstructorDecl *Constructor = cast<CXXConstructorDecl>(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 @@
                                 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 @@
   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 @@
 
       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 @@
                                                          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 @@
       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;

Modified: cfe/trunk/lib/Sema/SemaInit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.h?rev=101704&r1=101703&r2=101704&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.h (original)
+++ cfe/trunk/lib/Sema/SemaInit.h Sun Apr 18 02:40:54 2010
@@ -416,6 +416,10 @@
     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 @@
      
   /// \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,

Added: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp?rev=101704&view=auto
==============================================================================
--- cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp (added)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp Sun Apr 18 02:40:54 2010
@@ -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<typename T>
+T get_value_badly() {
+  double *dp = 0;
+  T *tp = dp; // FIXME: Should get an error here, from instantiating the
+              // default argument of X4<int>
+  return T();
+}
+
+template<typename T>
+struct X4 {
+  X4();
+  X4(const X4&, T = get_value_badly<T>());
+};
+
+void g1(const X1&);
+void g2(const X2&);
+void g3(const X3&);
+void g4(const X4<int>&);
+
+void test() {
+  g1(X1());
+  g2(X2());
+  g3(X3());
+  g4(X4<int>());
+}

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp
------------------------------------------------------------------------------
    svn:keywords = Id

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p16-cxx0x-no-extra-copy.cpp
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp?rev=101704&view=auto
==============================================================================
--- cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp (added)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp Sun Apr 18 02:40:54 2010
@@ -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<typename T>
+T get_value_badly() {
+  double *dp = 0;
+  T *tp = dp; // FIXME: Should get an error here, from instantiating the
+              // default argument of X4<int>
+  return T();
+}
+
+template<typename T>
+struct X4 {
+  X4();
+  X4(const X4&, T = get_value_badly<T>());
+};
+
+void g1(const X1&);
+void g2(const X2&);
+void g3(const X3&);
+void g4(const X4<int>&);
+
+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<int>());
+}

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
------------------------------------------------------------------------------
    svn:keywords = Id

Propchange: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
------------------------------------------------------------------------------
    svn:mime-type = text/plain





More information about the cfe-commits mailing list