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.

llvm-svn: 111357

clang/include/clang/Sema/Sema.h

@@ -2961,16 +2961,16 @@ public:
     AbstractReturnType,
     AbstractParamType,
     AbstractVariableType,
-    AbstractFieldType
+    AbstractFieldType,
+    AbstractArrayType
   };
 
   bool RequireNonAbstractType(SourceLocation Loc, QualType T,
-                              const PartialDiagnostic &PD,
+                              const PartialDiagnostic &PD);
-                              const CXXRecordDecl *CurrentRD = 0);
+  void DiagnoseAbstractType(const CXXRecordDecl *RD);
 
   bool RequireNonAbstractType(SourceLocation Loc, QualType T, unsigned DiagID,
-                              AbstractDiagSelID SelID = AbstractNone,
+                              AbstractDiagSelID SelID = AbstractNone);
-                              const CXXRecordDecl *CurrentRD = 0);
 
   //===--------------------------------------------------------------------===//
   // C++ Overloaded Operators [C++ 13.5]

clang/lib/Sema/SemaDeclCXX.cpp

@@ -2311,25 +2311,20 @@ void Sema::ActOnDefaultCtorInitializers(DeclPtrTy CDtorDecl) {
 }
 
 bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
-                                  unsigned DiagID, AbstractDiagSelID SelID,
+                                  unsigned DiagID, AbstractDiagSelID SelID) {
-                                  const CXXRecordDecl *CurrentRD) {
   if (SelID == -1)
-    return RequireNonAbstractType(Loc, T,
+    return RequireNonAbstractType(Loc, T, PDiag(DiagID));
-                                  PDiag(DiagID), CurrentRD);
   else
-    return RequireNonAbstractType(Loc, T,
+    return RequireNonAbstractType(Loc, T, PDiag(DiagID) << SelID);
-                                  PDiag(DiagID) << SelID, CurrentRD);
 }
 
 bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
-                                  const PartialDiagnostic &PD,
+                                  const PartialDiagnostic &PD) {
-                                  const CXXRecordDecl *CurrentRD) {
   if (!getLangOptions().CPlusPlus)
     return false;
 
   if (const ArrayType *AT = Context.getAsArrayType(T))
-    return RequireNonAbstractType(Loc, AT->getElementType(), PD,
+    return RequireNonAbstractType(Loc, AT->getElementType(), PD);
-                                  CurrentRD);
 
   if (const PointerType *PT = T->getAs<PointerType>()) {
     // Find the innermost pointer type.
@@ -2337,7 +2332,7 @@ bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
       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 @@ bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
 
   const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
 
-  if (CurrentRD && CurrentRD != RD)
+  // We can't answer whether something is abstract until it has a
-    return false;
+  // definition.  If it's currently being defined, we'll walk back
-
+  // over all the declarations when we have a full definition.
-  // FIXME: is this reasonable?  It matches current behavior, but....
+  const CXXRecordDecl *Def = RD->getDefinition();
-  if (!RD->getDefinition())
+  if (!Def || Def->isBeingDefined())
     return false;
 
   if (!RD->isAbstract())
     return false;
 
   Diag(Loc, PD) << RD->getDeclName();
+  DiagnoseAbstractType(RD);
 
-  // Check if we've already emitted the list of pure virtual functions for this
+  return true;
-  // 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 @@ bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
   if (!PureVirtualClassDiagSet)
     PureVirtualClassDiagSet.reset(new RecordDeclSetTy);
   PureVirtualClassDiagSet->insert(RD);
-
-  return true;
 }
 
 namespace {
-  class AbstractClassUsageDiagnoser
+struct AbstractUsageInfo {
-    : public DeclVisitor<AbstractClassUsageDiagnoser, bool> {
+  Sema &S;
-    Sema &SemaRef;
+  CXXRecordDecl *Record;
-    CXXRecordDecl *AbstractClass;
+  CanQualType AbstractType;
+  bool Invalid;
 
-    bool VisitDeclContext(const DeclContext *DC) {
+  AbstractUsageInfo(Sema &S, CXXRecordDecl *Record)
-      bool Invalid = false;
+    : S(S), Record(Record),
+      AbstractType(S.Context.getCanonicalType(
+                   S.Context.getTypeDeclType(Record))),
+      Invalid(false) {}
 
-      for (CXXRecordDecl::decl_iterator I = DC->decls_begin(),
+  void DiagnoseAbstractType() {
-           E = DC->decls_end(); I != E; ++I)
+    if (Invalid) return;
-        Invalid |= Visit(*I);
+    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:
+  void Check(FunctionProtoTypeLoc TL, Sema::AbstractDiagSelID Sel) {
-    AbstractClassUsageDiagnoser(Sema& SemaRef, CXXRecordDecl *ac)
+    Visit(TL.getResultLoc(), Sema::AbstractReturnType);
-      : SemaRef(SemaRef), AbstractClass(ac) {
+    for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
-        Visit(SemaRef.Context.getTranslationUnitDecl());
+      TypeSourceInfo *TSI = TL.getArg(I)->getTypeSourceInfo();
+      if (TSI) Visit(TSI->getTypeLoc(), Sema::AbstractParamType);
     }
+  }
 
-    bool VisitFunctionDecl(const FunctionDecl *FD) {
+  void Check(ArrayTypeLoc TL, Sema::AbstractDiagSelID Sel) {
-      if (FD->isThisDeclarationADefinition()) {
+    Visit(TL.getElementLoc(), Sema::AbstractArrayType);
-        // 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);
-      }
 
-      // Check the return type.
+  void Check(TemplateSpecializationTypeLoc TL, Sema::AbstractDiagSelID Sel) {
-      QualType RTy = FD->getType()->getAs<FunctionType>()->getResultType();
+    // Visit the type parameters from a permissive context.
-      bool Invalid =
+    for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
-        SemaRef.RequireNonAbstractType(FD->getLocation(), RTy,
+      TemplateArgumentLoc TAL = TL.getArgLoc(I);
-                                       diag::err_abstract_type_in_decl,
+      if (TAL.getArgument().getKind() == TemplateArgument::Type)
-                                       Sema::AbstractReturnType,
+        if (TypeSourceInfo *TSI = TAL.getTypeSourceInfo())
-                                       AbstractClass);
+          Visit(TSI->getTypeLoc(), Sema::AbstractNone);
-
+      // TODO: other template argument types?
-      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);
-      }
-
-      return Invalid;
     }
+  }
 
-    bool VisitDecl(const Decl* D) {
+  // Visit pointee types from a permissive context.
-      if (const DeclContext *DC = dyn_cast<DeclContext>(D))
+#define CheckPolymorphic(Type) \
-        return VisitDeclContext(DC);
+  void Check(Type TL, Sema::AbstractDiagSelID Sel) { \
+    Visit(TL.getNextTypeLoc(), Sema::AbstractNone); \
+  }
+  CheckPolymorphic(PointerTypeLoc)
+  CheckPolymorphic(ReferenceTypeLoc)
+  CheckPolymorphic(MemberPointerTypeLoc)
+  CheckPolymorphic(BlockPointerTypeLoc)
 
-      return false;
+  /// 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();
+  }
+};
+
+void AbstractUsageInfo::CheckType(const NamedDecl *D, TypeLoc TL,
+                                  Sema::AbstractDiagSelID Sel) {
+  CheckAbstractUsage(*this, D).Visit(TL, Sel);
+}
+
+}
+
+/// 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 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
     }
   }
 
-  if (Record->isAbstract() && !Record->isInvalidDecl())
+  if (Record->isAbstract() && !Record->isInvalidDecl()) {
-    (void)AbstractClassUsageDiagnoser(*this, Record);
+    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

clang/test/SemaCXX/abstract.cpp

@@ -67,20 +67,23 @@ class F {
   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 @@ namespace test1 {
     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>'}}
+  };
+}