From 26a006d892da9adf66feda475a2cf5f62973269d Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 15 Dec 2016 02:28:18 +0000 Subject: [PATCH] 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! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@289753 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaExprCXX.cpp | 7 -- lib/Sema/SemaInit.cpp | 30 +++++- lib/Sema/SemaObjCProperty.cpp | 6 +- test/CXX/class.derived/class.abstract/p2.cpp | 23 +++++ test/CXX/class.derived/class.abstract/p3.cpp | 97 ++++++++++++++++++++ 5 files changed, 153 insertions(+), 10 deletions(-) create mode 100644 test/CXX/class.derived/class.abstract/p2.cpp create mode 100644 test/CXX/class.derived/class.abstract/p3.cpp diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index a4546cec5a..74beeac724 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/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, diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 6ece7c8d78..ce012896f5 100644 --- a/lib/Sema/SemaInit.cpp +++ b/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(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. diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index 9ab80b4f79..3481b82679 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/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); diff --git a/test/CXX/class.derived/class.abstract/p2.cpp b/test/CXX/class.derived/class.abstract/p2.cpp new file mode 100644 index 0000000000..713a5269f1 --- /dev/null +++ b/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 +}; diff --git a/test/CXX/class.derived/class.abstract/p3.cpp b/test/CXX/class.derived/class.abstract/p3.cpp new file mode 100644 index 0000000000..ad5b874788 --- /dev/null +++ b/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 struct SecretlyAbstract { + SecretlyAbstract(); + SecretlyAbstract(int); + virtual void f() = 0; // expected-note 1+{{unimplemented}} +}; +using B = SecretlyAbstract; +using C = SecretlyAbstract; +using D = SecretlyAbstract[1]; + +B b; // expected-error {{abstract class}} +D d; // expected-error {{abstract class}} + +template 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 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 struct initializer_list { + const T *begin, *end; + initializer_list(); + }; +} +std::initializer_list ila = {1, 2, 3, 4}; // expected-error {{abstract class}} -- 2.40.0