[cfe-commits] r111357 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp test/SemaCXX/abstract.cpp
John McCall
rjmccall at apple.com
Wed Aug 18 02:41:07 PDT 2010
Author: rjmccall
Date: Wed Aug 18 04:41:07 2010
New Revision: 111357
URL: http://llvm.org/viewvc/llvm-project?rev=111357&view=rev
Log:
Rip out the existing retroactive abstract-class usage checker,
which in a fit of zeal wanted to walk the entire translation unit,
and replace it with a new checker that walks the types of declarations
nested within the class. Also, look into templates when doing this.
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/abstract.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=111357&r1=111356&r2=111357&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Aug 18 04:41:07 2010
@@ -2961,16 +2961,16 @@
AbstractReturnType,
AbstractParamType,
AbstractVariableType,
- AbstractFieldType
+ AbstractFieldType,
+ AbstractArrayType
};
bool RequireNonAbstractType(SourceLocation Loc, QualType T,
- const PartialDiagnostic &PD,
- const CXXRecordDecl *CurrentRD = 0);
+ const PartialDiagnostic &PD);
+ void DiagnoseAbstractType(const CXXRecordDecl *RD);
bool RequireNonAbstractType(SourceLocation Loc, QualType T, unsigned DiagID,
- AbstractDiagSelID SelID = AbstractNone,
- const CXXRecordDecl *CurrentRD = 0);
+ AbstractDiagSelID SelID = AbstractNone);
//===--------------------------------------------------------------------===//
// C++ Overloaded Operators [C++ 13.5]
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=111357&r1=111356&r2=111357&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Aug 18 04:41:07 2010
@@ -2311,25 +2311,20 @@
}
bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
- unsigned DiagID, AbstractDiagSelID SelID,
- const CXXRecordDecl *CurrentRD) {
+ unsigned DiagID, AbstractDiagSelID SelID) {
if (SelID == -1)
- return RequireNonAbstractType(Loc, T,
- PDiag(DiagID), CurrentRD);
+ return RequireNonAbstractType(Loc, T, PDiag(DiagID));
else
- return RequireNonAbstractType(Loc, T,
- PDiag(DiagID) << SelID, CurrentRD);
+ return RequireNonAbstractType(Loc, T, PDiag(DiagID) << SelID);
}
bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
- const PartialDiagnostic &PD,
- const CXXRecordDecl *CurrentRD) {
+ const PartialDiagnostic &PD) {
if (!getLangOptions().CPlusPlus)
return false;
if (const ArrayType *AT = Context.getAsArrayType(T))
- return RequireNonAbstractType(Loc, AT->getElementType(), PD,
- CurrentRD);
+ return RequireNonAbstractType(Loc, AT->getElementType(), PD);
if (const PointerType *PT = T->getAs<PointerType>()) {
// Find the innermost pointer type.
@@ -2337,7 +2332,7 @@
PT = T;
if (const ArrayType *AT = Context.getAsArrayType(PT->getPointeeType()))
- return RequireNonAbstractType(Loc, AT->getElementType(), PD, CurrentRD);
+ return RequireNonAbstractType(Loc, AT->getElementType(), PD);
}
const RecordType *RT = T->getAs<RecordType>();
@@ -2346,22 +2341,27 @@
const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
- if (CurrentRD && CurrentRD != RD)
- return false;
-
- // FIXME: is this reasonable? It matches current behavior, but....
- if (!RD->getDefinition())
+ // We can't answer whether something is abstract until it has a
+ // definition. If it's currently being defined, we'll walk back
+ // over all the declarations when we have a full definition.
+ const CXXRecordDecl *Def = RD->getDefinition();
+ if (!Def || Def->isBeingDefined())
return false;
if (!RD->isAbstract())
return false;
Diag(Loc, PD) << RD->getDeclName();
+ DiagnoseAbstractType(RD);
+
+ return true;
+}
- // Check if we've already emitted the list of pure virtual functions for this
- // class.
+void Sema::DiagnoseAbstractType(const CXXRecordDecl *RD) {
+ // Check if we've already emitted the list of pure virtual functions
+ // for this class.
if (PureVirtualClassDiagSet && PureVirtualClassDiagSet->count(RD))
- return true;
+ return;
CXXFinalOverriderMap FinalOverriders;
RD->getFinalOverriders(FinalOverriders);
@@ -2401,69 +2401,175 @@
if (!PureVirtualClassDiagSet)
PureVirtualClassDiagSet.reset(new RecordDeclSetTy);
PureVirtualClassDiagSet->insert(RD);
-
- return true;
}
namespace {
- class AbstractClassUsageDiagnoser
- : public DeclVisitor<AbstractClassUsageDiagnoser, bool> {
- Sema &SemaRef;
- CXXRecordDecl *AbstractClass;
-
- bool VisitDeclContext(const DeclContext *DC) {
- bool Invalid = false;
-
- for (CXXRecordDecl::decl_iterator I = DC->decls_begin(),
- E = DC->decls_end(); I != E; ++I)
- Invalid |= Visit(*I);
+struct AbstractUsageInfo {
+ Sema &S;
+ CXXRecordDecl *Record;
+ CanQualType AbstractType;
+ bool Invalid;
+
+ AbstractUsageInfo(Sema &S, CXXRecordDecl *Record)
+ : S(S), Record(Record),
+ AbstractType(S.Context.getCanonicalType(
+ S.Context.getTypeDeclType(Record))),
+ Invalid(false) {}
+
+ void DiagnoseAbstractType() {
+ if (Invalid) return;
+ S.DiagnoseAbstractType(Record);
+ Invalid = true;
+ }
- return Invalid;
+ bool HasAbstractType(QualType T) {
+ if (T->isArrayType())
+ T = S.Context.getBaseElementType(T);
+ CanQualType CT = T->getCanonicalTypeUnqualified().getUnqualifiedType();
+ return (CT == AbstractType);
+ }
+
+ void CheckType(const NamedDecl *D, TypeLoc TL, Sema::AbstractDiagSelID Sel);
+};
+
+struct CheckAbstractUsage {
+ AbstractUsageInfo &Info;
+ const NamedDecl *Ctx;
+
+ CheckAbstractUsage(AbstractUsageInfo &Info, const NamedDecl *Ctx)
+ : Info(Info), Ctx(Ctx) {}
+
+ void Visit(TypeLoc TL, Sema::AbstractDiagSelID Sel) {
+ switch (TL.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+ case TypeLoc::CLASS: Check(cast<CLASS##TypeLoc>(TL), Sel); break;
+#include "clang/AST/TypeLocNodes.def"
}
+ }
- public:
- AbstractClassUsageDiagnoser(Sema& SemaRef, CXXRecordDecl *ac)
- : SemaRef(SemaRef), AbstractClass(ac) {
- Visit(SemaRef.Context.getTranslationUnitDecl());
+ void Check(FunctionProtoTypeLoc TL, Sema::AbstractDiagSelID Sel) {
+ Visit(TL.getResultLoc(), Sema::AbstractReturnType);
+ for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+ TypeSourceInfo *TSI = TL.getArg(I)->getTypeSourceInfo();
+ if (TSI) Visit(TSI->getTypeLoc(), Sema::AbstractParamType);
}
+ }
- bool VisitFunctionDecl(const FunctionDecl *FD) {
- if (FD->isThisDeclarationADefinition()) {
- // No need to do the check if we're in a definition, because it requires
- // that the return/param types are complete.
- // because that requires
- return VisitDeclContext(FD);
- }
+ void Check(ArrayTypeLoc TL, Sema::AbstractDiagSelID Sel) {
+ Visit(TL.getElementLoc(), Sema::AbstractArrayType);
+ }
- // Check the return type.
- QualType RTy = FD->getType()->getAs<FunctionType>()->getResultType();
- bool Invalid =
- SemaRef.RequireNonAbstractType(FD->getLocation(), RTy,
- diag::err_abstract_type_in_decl,
- Sema::AbstractReturnType,
- AbstractClass);
-
- for (FunctionDecl::param_const_iterator I = FD->param_begin(),
- E = FD->param_end(); I != E; ++I) {
- const ParmVarDecl *VD = *I;
- Invalid |=
- SemaRef.RequireNonAbstractType(VD->getLocation(),
- VD->getOriginalType(),
- diag::err_abstract_type_in_decl,
- Sema::AbstractParamType,
- AbstractClass);
- }
+ void Check(TemplateSpecializationTypeLoc TL, Sema::AbstractDiagSelID Sel) {
+ // Visit the type parameters from a permissive context.
+ for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+ TemplateArgumentLoc TAL = TL.getArgLoc(I);
+ if (TAL.getArgument().getKind() == TemplateArgument::Type)
+ if (TypeSourceInfo *TSI = TAL.getTypeSourceInfo())
+ Visit(TSI->getTypeLoc(), Sema::AbstractNone);
+ // TODO: other template argument types?
+ }
+ }
+
+ // Visit pointee types from a permissive context.
+#define CheckPolymorphic(Type) \
+ void Check(Type TL, Sema::AbstractDiagSelID Sel) { \
+ Visit(TL.getNextTypeLoc(), Sema::AbstractNone); \
+ }
+ CheckPolymorphic(PointerTypeLoc)
+ CheckPolymorphic(ReferenceTypeLoc)
+ CheckPolymorphic(MemberPointerTypeLoc)
+ CheckPolymorphic(BlockPointerTypeLoc)
- return Invalid;
+ /// Handle all the types we haven't given a more specific
+ /// implementation for above.
+ void Check(TypeLoc TL, Sema::AbstractDiagSelID Sel) {
+ // Every other kind of type that we haven't called out already
+ // that has an inner type is either (1) sugar or (2) contains that
+ // inner type in some way as a subobject.
+ if (TypeLoc Next = TL.getNextTypeLoc())
+ return Visit(Next, Sel);
+
+ // If there's no inner type and we're in a permissive context,
+ // don't diagnose.
+ if (Sel == Sema::AbstractNone) return;
+
+ // Check whether the type matches the abstract type.
+ QualType T = TL.getType();
+ if (T->isArrayType()) {
+ Sel = Sema::AbstractArrayType;
+ T = Info.S.Context.getBaseElementType(T);
+ }
+ CanQualType CT = T->getCanonicalTypeUnqualified().getUnqualifiedType();
+ if (CT != Info.AbstractType) return;
+
+ // It matched; do some magic.
+ if (Sel == Sema::AbstractArrayType) {
+ Info.S.Diag(Ctx->getLocation(), diag::err_array_of_abstract_type)
+ << T << TL.getSourceRange();
+ } else {
+ Info.S.Diag(Ctx->getLocation(), diag::err_abstract_type_in_decl)
+ << Sel << T << TL.getSourceRange();
}
+ Info.DiagnoseAbstractType();
+ }
+};
- bool VisitDecl(const Decl* D) {
- if (const DeclContext *DC = dyn_cast<DeclContext>(D))
- return VisitDeclContext(DC);
+void AbstractUsageInfo::CheckType(const NamedDecl *D, TypeLoc TL,
+ Sema::AbstractDiagSelID Sel) {
+ CheckAbstractUsage(*this, D).Visit(TL, Sel);
+}
- return false;
+}
+
+/// Check for invalid uses of an abstract type in a method declaration.
+static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
+ CXXMethodDecl *MD) {
+ // No need to do the check on definitions, which require that
+ // the return/param types be complete.
+ if (MD->isThisDeclarationADefinition())
+ return;
+
+ // For safety's sake, just ignore it if we don't have type source
+ // information. This should never happen for non-implicit methods,
+ // but...
+ if (TypeSourceInfo *TSI = MD->getTypeSourceInfo())
+ Info.CheckType(MD, TSI->getTypeLoc(), Sema::AbstractNone);
+}
+
+/// Check for invalid uses of an abstract type within a class definition.
+static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
+ CXXRecordDecl *RD) {
+ for (CXXRecordDecl::decl_iterator
+ I = RD->decls_begin(), E = RD->decls_end(); I != E; ++I) {
+ Decl *D = *I;
+ if (D->isImplicit()) continue;
+
+ // Methods and method templates.
+ if (isa<CXXMethodDecl>(D)) {
+ CheckAbstractClassUsage(Info, cast<CXXMethodDecl>(D));
+ } else if (isa<FunctionTemplateDecl>(D)) {
+ FunctionDecl *FD = cast<FunctionTemplateDecl>(D)->getTemplatedDecl();
+ CheckAbstractClassUsage(Info, cast<CXXMethodDecl>(FD));
+
+ // Fields and static variables.
+ } else if (isa<FieldDecl>(D)) {
+ FieldDecl *FD = cast<FieldDecl>(D);
+ if (TypeSourceInfo *TSI = FD->getTypeSourceInfo())
+ Info.CheckType(FD, TSI->getTypeLoc(), Sema::AbstractFieldType);
+ } else if (isa<VarDecl>(D)) {
+ VarDecl *VD = cast<VarDecl>(D);
+ if (TypeSourceInfo *TSI = VD->getTypeSourceInfo())
+ Info.CheckType(VD, TSI->getTypeLoc(), Sema::AbstractVariableType);
+
+ // Nested classes and class templates.
+ } else if (isa<CXXRecordDecl>(D)) {
+ CheckAbstractClassUsage(Info, cast<CXXRecordDecl>(D));
+ } else if (isa<ClassTemplateDecl>(D)) {
+ CheckAbstractClassUsage(Info,
+ cast<ClassTemplateDecl>(D)->getTemplatedDecl());
}
- };
+ }
}
/// \brief Perform semantic checks on a class definition that has been
@@ -2548,8 +2654,10 @@
}
}
- if (Record->isAbstract() && !Record->isInvalidDecl())
- (void)AbstractClassUsageDiagnoser(*this, Record);
+ if (Record->isAbstract() && !Record->isInvalidDecl()) {
+ AbstractUsageInfo Info(*this, Record);
+ CheckAbstractClassUsage(Info, Record);
+ }
// If this is not an aggregate type and has no user-declared constructor,
// complain about any non-static data members of reference or const scalar
Modified: cfe/trunk/test/SemaCXX/abstract.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/abstract.cpp?rev=111357&r1=111356&r2=111357&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/abstract.cpp (original)
+++ cfe/trunk/test/SemaCXX/abstract.cpp Wed Aug 18 04:41:07 2010
@@ -67,20 +67,23 @@
virtual void f() = 0; // expected-note {{pure virtual function 'f'}}
};
+// Diagnosing in these cases is prohibitively expensive. We still
+// diagnose at the function definition, of course.
+
class Abstract;
-void t7(Abstract a); // expected-error {{parameter type 'Abstract' is an abstract class}}
+void t7(Abstract a);
void t8() {
- void h(Abstract a); // expected-error {{parameter type 'Abstract' is an abstract class}}
+ void h(Abstract a);
}
namespace N {
-void h(Abstract a); // expected-error {{parameter type 'Abstract' is an abstract class}}
+void h(Abstract a);
}
class Abstract {
- virtual void f() = 0; // expected-note {{pure virtual function 'f'}}
+ virtual void f() = 0;
};
// <rdar://problem/6854087>
@@ -186,3 +189,19 @@
C c;
}
}
+
+// rdar://problem/8302168
+namespace test2 {
+ struct X1 {
+ virtual void xfunc(void) = 0; // expected-note {{pure virtual function}}
+ void g(X1 parm7); // expected-error {{parameter type 'test2::X1' is an abstract class}}
+ void g(X1 parm8[2]); // expected-error {{array of abstract class type 'test2::X1'}}
+ };
+
+ template <int N>
+ struct X2 {
+ virtual void xfunc(void) = 0; // expected-note {{pure virtual function}}
+ void g(X2 parm10); // expected-error {{parameter type 'X2<N>' is an abstract class}}
+ void g(X2 parm11[2]); // expected-error {{array of abstract class type 'X2<N>'}}
+ };
+}
More information about the cfe-commits
mailing list