Move checks for creation of objects of abstract class type from the various

constructs that can do so into the initialization code. This fixes a number
of different cases in which we used to fail to check for abstract types.

Thanks to Tim Shen for inspiring the weird code that uncovered this!

llvm-svn: 289753

clang/lib/Sema/SemaExprCXX.cpp

@@ -1291,10 +1291,6 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
                           diag::err_invalid_incomplete_type_use, FullRange))
     return ExprError();
 
-  if (RequireNonAbstractType(TyBeginLoc, Ty,
-                             diag::err_allocation_of_abstract_type))
-    return ExprError();
-
   InitializedEntity Entity = InitializedEntity::InitializeTemporary(TInfo);
   InitializationKind Kind =
       Exprs.size() ? ListInitialization
@@ -5491,9 +5487,6 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
   if (Context.getCanonicalType(LTy) == Context.getCanonicalType(RTy)) {
     if (LTy->isRecordType()) {
       // The operands have class type. Make a temporary copy.
-      if (RequireNonAbstractType(QuestionLoc, LTy,
-                                 diag::err_allocation_of_abstract_type))
-        return QualType();
       InitializedEntity Entity = InitializedEntity::InitializeTemporary(LTy);
 
       ExprResult LHSCopy = PerformCopyInitialization(Entity,

clang/lib/Sema/SemaInit.cpp

@@ -4599,7 +4599,7 @@ static void TryValueInitialization(Sema &S,
       MultiExprArg Args(&InitListAsExpr, InitList ? 1 : 0);
       bool InitListSyntax = InitList;
 
-      // FIXME: Instead of creating a CXXConstructExpr of non-array type here,
+      // FIXME: Instead of creating a CXXConstructExpr of array type here,
       // wrap a class-typed CXXConstructExpr in an ArrayInitLoopExpr.
       return TryConstructorInitialization(
           S, Entity, Kind, Args, T, Entity.getType(), Sequence, InitListSyntax);
@@ -6366,6 +6366,8 @@ ExprResult Sema::TemporaryMaterializationConversion(Expr *E) {
     return E;
 
   // C++1z [conv.rval]/1: T shall be a complete type.
+  // FIXME: Does this ever matter (can we form a prvalue of incomplete type)?
+  // If so, we should check for a non-abstract class type here too.
   QualType T = E->getType();
   if (RequireCompleteType(E->getExprLoc(), T, diag::err_incomplete_type))
     return ExprError();
@@ -6541,6 +6543,17 @@ InitializationSequence::Perform(Sema &S,
     break;
   }
 
+  // C++ [class.abstract]p2:
+  //   no objects of an abstract class can be created except as subobjects
+  //   of a class derived from it
+  auto checkAbstractType = [&](QualType T) -> bool {
+    if (Entity.getKind() == InitializedEntity::EK_Base ||
+        Entity.getKind() == InitializedEntity::EK_Delegating)
+      return false;
+    return S.RequireNonAbstractType(Kind.getLocation(), T,
+                                    diag::err_allocation_of_abstract_type);
+  };
+
   // Walk through the computed steps for the initialization sequence,
   // performing the specified conversions along the way.
   bool ConstructorInitRequiresZeroInit = false;
@@ -6647,6 +6660,9 @@ InitializationSequence::Perform(Sema &S,
     }
 
     case SK_FinalCopy:
+      if (checkAbstractType(Step->Type))
+        return ExprError();
+
       // If the overall initialization is initializing a temporary, we already
       // bound our argument if it was necessary to do so. If not (if we're
       // ultimately initializing a non-temporary), our argument needs to be
@@ -6731,6 +6747,9 @@ InitializationSequence::Perform(Sema &S,
         CreatedObject = Conversion->getReturnType()->isRecordType();
       }
 
+      if (CreatedObject && checkAbstractType(CurInit.get()->getType()))
+        return ExprError();
+
       CurInit = ImplicitCastExpr::Create(S.Context, CurInit.get()->getType(),
                                          CastKind, CurInit.get(), nullptr,
                                          CurInit.get()->getValueKind());
@@ -6813,6 +6832,9 @@ InitializationSequence::Perform(Sema &S,
     }
 
     case SK_ListInitialization: {
+      if (checkAbstractType(Step->Type))
+        return ExprError();
+
       InitListExpr *InitList = cast<InitListExpr>(CurInit.get());
       // If we're not initializing the top-level entity, we need to create an
       // InitializeTemporary entity for our target type.
@@ -6849,6 +6871,9 @@ InitializationSequence::Perform(Sema &S,
     }
 
     case SK_ConstructorInitializationFromList: {
+      if (checkAbstractType(Step->Type))
+        return ExprError();
+
       // When an initializer list is passed for a parameter of type "reference
       // to object", we don't get an EK_Temporary entity, but instead an
       // EK_Parameter entity with reference type.
@@ -6892,6 +6917,9 @@ InitializationSequence::Perform(Sema &S,
 
     case SK_ConstructorInitialization:
     case SK_StdInitializerListConstructorCall: {
+      if (checkAbstractType(Step->Type))
+        return ExprError();
+
       // When an initializer list is passed for a parameter of type "reference
       // to object", we don't get an EK_Temporary entity, but instead an
       // EK_Parameter entity with reference type.

clang/lib/Sema/SemaObjCProperty.cpp

@@ -1146,9 +1146,11 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S,
                                  diag::err_abstract_type_in_decl,
                                  AbstractSynthesizedIvarType)) {
         Diag(property->getLocation(), diag::note_property_declare);
+        // An abstract type is as bad as an incomplete type.
+        CompleteTypeErr = true;
+      }
+      if (CompleteTypeErr)
         Ivar->setInvalidDecl();
-      } else if (CompleteTypeErr)
-          Ivar->setInvalidDecl();
       ClassImpDecl->addDecl(Ivar);
       IDecl->makeDeclVisibleInContext(Ivar);
 

clang/test/CXX/class.derived/class.abstract/p2.cpp

@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++1z -verify %s
+
+// no objects of an abstract class can be created except as subobjects of a
+// class derived from it
+
+struct A {
+  A() {}
+  A(int) : A() {} // ok
+
+  virtual void f() = 0; // expected-note 1+{{unimplemented}}
+};
+
+void f(A &&a);
+
+void g() {
+  f({}); // expected-error {{abstract class}}
+  f({0}); // expected-error {{abstract class}}
+  f(0); // expected-error {{abstract class}}
+}
+
+struct B : A {
+  B() : A() {} // ok
+};

clang/test/CXX/class.derived/class.abstract/p3.cpp

@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -std=c++1z -verify %s
+
+struct A {
+  A() {}
+  A(int) : A() {} // ok
+
+  virtual void f() = 0; // expected-note 1+{{unimplemented}}
+};
+
+template<typename> struct SecretlyAbstract {
+  SecretlyAbstract();
+  SecretlyAbstract(int);
+  virtual void f() = 0; // expected-note 1+{{unimplemented}}
+};
+using B = SecretlyAbstract<int>;
+using C = SecretlyAbstract<float>;
+using D = SecretlyAbstract<char>[1];
+
+B b; // expected-error {{abstract class}}
+D d; // expected-error {{abstract class}}
+
+template<int> struct N;
+
+// Note: C is not instantiated anywhere in this file, so we never discover that
+// it is in fact abstract. The C++ standard suggests that we need to
+// instantiate in all cases where abstractness could affect the validity of a
+// program, but that breaks a *lot* of code, so we don't do that.
+//
+// FIXME: Once DR1640 is resolved, remove the check on forming an abstract
+// array type entirely. The only restriction we need is that you can't create
+// an object of abstract (most-derived) type.
+
+
+// An abstract class shall not be used
+
+//  - as a parameter type
+void f(A&);
+void f(A); // expected-error {{abstract class}}
+void f(A[1]); // expected-error {{abstract class}}
+void f(B); // expected-error {{abstract class}}
+void f(B[1]); // expected-error {{abstract class}}
+void f(C);
+void f(C[1]);
+void f(D); // expected-error {{abstract class}}
+void f(D[1]); // expected-error {{abstract class}}
+
+//  - as a function return type
+A &f(N<0>);
+A *f(N<1>);
+A f(N<2>); // expected-error {{abstract class}}
+A (&f(N<3>))[2]; // expected-error {{abstract class}}
+B f(N<4>); // expected-error {{abstract class}}
+B (&f(N<5>))[2]; // expected-error {{abstract class}}
+C f(N<6>);
+C (&f(N<7>))[2];
+
+//  - as the type of an explicit conversion
+void g(A&&);
+void h() {
+  A(); // expected-error {{abstract class}}
+  A(0); // expected-error {{abstract class}}
+  A{}; // expected-error {{abstract class}}
+  A{0}; // expected-error {{abstract class}}
+  (A)(0); // expected-error {{abstract class}}
+  (A){}; // expected-error {{abstract class}}
+  (A){0}; // expected-error {{abstract class}}
+
+  D(); // expected-error {{array type}}
+  D{}; // expected-error {{abstract class}}
+  D{0}; // expected-error {{abstract class}}
+  (D){}; // expected-error {{abstract class}}
+  (D){0}; // expected-error {{abstract class}}
+}
+
+template<typename T> void t(T); // expected-note 2{{abstract class}}
+void i(A &a, B &b, C &c, D &d) {
+  // FIXME: These should be handled consistently. We currently reject the first
+  // two early because we (probably incorrectly, depending on dr1640) take
+  // abstractness into account in forming implicit conversion sequences.
+  t(a); // expected-error {{no matching function}}
+  t(b); // expected-error {{no matching function}}
+  t(c); // expected-error {{allocating an object of abstract class type}}
+  t(d); // ok, decays to pointer
+}
+
+struct E : A {
+  E() : A() {} // ok
+  E(int n) : A( A(n) ) {} // expected-error {{abstract class}}
+};
+
+namespace std {
+  template<typename T> struct initializer_list {
+    const T *begin, *end;
+    initializer_list();
+  };
+}
+std::initializer_list<A> ila = {1, 2, 3, 4}; // expected-error {{abstract class}}