[cfe-commits] r99351 - in /cfe/trunk: include/clang/AST/CXXInheritance.h include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/CXXInheritance.cpp lib/Sema/SemaDeclCXX.cpp test/CXX/class.derived/class.abstract/p4.cpp test/CXX/class.derived/class.abstract/p5.cpp test/CXX/class.derived/class.virtual/p2.cpp

Jason Haslam jason.haslam at gmail.com
Tue Mar 23 23:08:12 PDT 2010


On Mar 23, 2010, at 5:47 PM, Douglas Gregor wrote:

> Author: dgregor
> Date: Tue Mar 23 18:47:56 2010
> New Revision: 99351
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=99351&view=rev
> Log:
> Implement computation of the final overriders for each virtual
> function within a class hierarchy (C++ [class.virtual]p2).
> 
> We use the final-overrider computation to determine when a particular
> class is ill-formed because it has multiple final overriders for a
> given virtual function (e.g., because two virtual functions override
> the same virtual function in the same virtual base class). Fixes
> PR5973.
> 
> We also use the final-overrider computation to determine which virtual
> member functions are pure when determining whether a class is
> abstract or diagnosing the improper use of an abstract class. The
> prior approach to determining whether there were any pure virtual
> functions in a class didn't cope with virtual base class subobjects
> properly, and could not easily be fixed to deal with the oddities of
> subobject hiding. Fixes PR6631.

This also fixes PR6124 which is the same as PR6631.

Jason

> 
> 
> Added:
>    cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp   (with props)
>    cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp   (with props)
>    cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp   (with props)
> Modified:
>    cfe/trunk/include/clang/AST/CXXInheritance.h
>    cfe/trunk/include/clang/AST/DeclCXX.h
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/AST/CXXInheritance.cpp
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> 
> Modified: cfe/trunk/include/clang/AST/CXXInheritance.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CXXInheritance.h?rev=99351&r1=99350&r2=99351&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CXXInheritance.h (original)
> +++ cfe/trunk/include/clang/AST/CXXInheritance.h Tue Mar 23 18:47:56 2010
> @@ -19,6 +19,7 @@
> #include "clang/AST/DeclCXX.h"
> #include "clang/AST/Type.h"
> #include "clang/AST/TypeOrdering.h"
> +#include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/SmallVector.h"
> #include <list>
> #include <map>
> @@ -227,6 +228,137 @@
>   /// object.
>   void swap(CXXBasePaths &Other);
> };
> +
> +/// \brief Uniquely identifies a virtual method within a class
> +/// hierarchy by the method itself and a class subobject number.
> +struct UniqueVirtualMethod {
> +  UniqueVirtualMethod() : Method(0), Subobject(0), InVirtualSubobject(0) { }
> +
> +  UniqueVirtualMethod(CXXMethodDecl *Method, unsigned Subobject,
> +                      const CXXRecordDecl *InVirtualSubobject)
> +    : Method(Method), Subobject(Subobject), 
> +      InVirtualSubobject(InVirtualSubobject) { }
> +
> +  /// \brief The overriding virtual method.
> +  CXXMethodDecl *Method;
> +
> +  /// \brief The subobject in which the overriding virtual method
> +  /// resides.
> +  unsigned Subobject;
> +
> +  /// \brief The virtual base class subobject of which this overridden
> +  /// virtual method is a part. Note that this records the closest
> +  /// derived virtual base class subobject.
> +  const CXXRecordDecl *InVirtualSubobject;
> +
> +  friend bool operator==(const UniqueVirtualMethod &X,
> +                         const UniqueVirtualMethod &Y) {
> +    return X.Method == Y.Method && X.Subobject == Y.Subobject &&
> +      X.InVirtualSubobject == Y.InVirtualSubobject;
> +  }
> +
> +  friend bool operator!=(const UniqueVirtualMethod &X,
> +                         const UniqueVirtualMethod &Y) {
> +    return !(X == Y);
> +  }
> +};
> +
> +/// \brief The set of methods that override a given virtual method in
> +/// each subobject where it occurs.
> +///
> +/// The first part of the pair is the subobject in which the
> +/// overridden virtual function occurs, while the second part of the
> +/// pair is the virtual method that overrides it (including the
> +/// subobject in which that virtual function occurs).
> +class OverridingMethods {
> +  llvm::DenseMap<unsigned, llvm::SmallVector<UniqueVirtualMethod, 4> > 
> +    Overrides;
> +
> +public:
> +  // Iterate over the set of subobjects that have overriding methods.
> +  typedef llvm::DenseMap<unsigned, llvm::SmallVector<UniqueVirtualMethod, 4> >
> +            ::iterator iterator;
> +  typedef llvm::DenseMap<unsigned, llvm::SmallVector<UniqueVirtualMethod, 4> >
> +            ::const_iterator const_iterator;
> +  iterator begin() { return Overrides.begin(); }
> +  const_iterator begin() const { return Overrides.begin(); }
> +  iterator end() { return Overrides.end(); }
> +  const_iterator end() const { return Overrides.end(); }
> +  unsigned size() const { return Overrides.size(); }
> +
> +  // Iterate over the set of overriding virtual methods in a given
> +  // subobject.
> +  typedef llvm::SmallVector<UniqueVirtualMethod, 4>::iterator 
> +    overriding_iterator;
> +  typedef llvm::SmallVector<UniqueVirtualMethod, 4>::const_iterator
> +    overriding_const_iterator;
> +
> +  // Add a new overriding method for a particular subobject.
> +  void add(unsigned OverriddenSubobject, UniqueVirtualMethod Overriding);
> +
> +  // Add all of the overriding methods from "other" into overrides for
> +  // this method. Used when merging the overrides from multiple base
> +  // class subobjects.
> +  void add(const OverridingMethods &Other);
> +
> +  // Replace all overriding virtual methods in all subobjects with the
> +  // given virtual method.
> +  void replaceAll(UniqueVirtualMethod Overriding);
> +};
> +
> +/// \brief A mapping from each virtual member function to its set of
> +/// final overriders.
> +///
> +/// Within a class hierarchy for a given derived class, each virtual
> +/// member function in that hierarchy has one or more "final
> +/// overriders" (C++ [class.virtual]p2). A final overrider for a
> +/// virtual function "f" is the virtual function that will actually be
> +/// invoked when dispatching a call to "f" through the
> +/// vtable. Well-formed classes have a single final overrider for each
> +/// virtual function; in abstract classes, the final overrider for at
> +/// least one virtual function is a pure virtual function. Due to
> +/// multiple, virtual inheritance, it is possible for a class to have
> +/// more than one final overrider. Athough this is an error (per C++
> +/// [class.virtual]p2), it is not considered an error here: the final
> +/// overrider map can represent multiple final overriders for a
> +/// method, and it is up to the client to determine whether they are
> +/// problem. For example, the following class \c D has two final
> +/// overriders for the virtual function \c A::f(), one in \c C and one
> +/// in \c D:
> +///
> +/// \code
> +///   struct A { virtual void f(); };
> +///   struct B : virtual A { virtual void f(); };
> +///   struct C : virtual A { virtual void f(); };
> +///   struct D : B, C { };
> +/// \endcode
> +///
> +/// This data structure contaings a mapping from every virtual
> +/// function *that does not override an existing virtual function* and
> +/// in every subobject where that virtual function occurs to the set
> +/// of virtual functions that override it. Thus, the same virtual
> +/// function \c A::f can actually occur in multiple subobjects of type
> +/// \c A due to multiple inheritance, and may be overriden by
> +/// different virtual functions in each, as in the following example:
> +///
> +/// \code
> +///   struct A { virtual void f(); };
> +///   struct B : A { virtual void f(); };
> +///   struct C : A { virtual void f(); };
> +///   struct D : B, C { };
> +/// \endcode
> +///
> +/// Unlike in the previous example, where the virtual functions \c
> +/// B::f and \c C::f both overrode \c A::f in the same subobject of
> +/// type \c A, in this example the two virtual functions both override
> +/// \c A::f but in *different* subobjects of type A. This is
> +/// represented by numbering the subobjects in which the overridden
> +/// and the overriding virtual member functions are located. Subobject
> +/// 0 represents the virtua base class subobject of that type, while
> +/// subobject numbers greater than 0 refer to non-virtual base class
> +/// subobjects of that type.
> +class CXXFinalOverriderMap 
> +  : public llvm::DenseMap<const CXXMethodDecl *, OverridingMethods> { };
> 
> } // end namespace clang
> 
> 
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=99351&r1=99350&r2=99351&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Mar 23 18:47:56 2010
> @@ -33,6 +33,7 @@
> class CXXMethodDecl;
> class CXXRecordDecl;
> class CXXMemberLookupCriteria;
> +class CXXFinalOverriderMap;
> class FriendDecl;
> 
> /// \brief Represents any kind of function declaration, whether it is a
> @@ -879,7 +880,12 @@
>   static bool FindNestedNameSpecifierMember(const CXXBaseSpecifier *Specifier,
>                                             CXXBasePath &Path,
>                                             void *UserData);
> -  
> +
> +  /// \brief Retrieve the final overriders for each virtual member
> +  /// function in the class hierarchy where this class is the
> +  /// most-derived class in the class hierarchy.
> +  void getFinalOverriders(CXXFinalOverriderMap &FinaOverriders) const;
> +
>   /// viewInheritance - Renders and displays an inheritance diagram
>   /// for this C++ class and all of its base classes (transitively) using
>   /// GraphViz.
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=99351&r1=99350&r2=99351&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 23 18:47:56 2010
> @@ -391,7 +391,11 @@
>   "%select{return|parameter|variable|field}0 type %1 is an abstract class">;
> def err_allocation_of_abstract_type : Error<
>   "allocation of an object of abstract type %0">;
> -  
> +
> +def err_multiple_final_overriders : Error<
> +  "virtual function %q0 has more than one final overrider in %1">; 
> +def note_final_overrider : Note<"final overrider of %q0 in %1">;
> +
> def err_type_defined_in_type_specifier : Error<
>   "%0 can not be defined in a type specifier">;
> def err_type_defined_in_result_type : Error<
> 
> Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=99351&r1=99350&r2=99351&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
> +++ cfe/trunk/lib/AST/CXXInheritance.cpp Tue Mar 23 18:47:56 2010
> @@ -416,3 +416,240 @@
> 
>   return false;
> }
> +
> +void OverridingMethods::add(unsigned OverriddenSubobject, 
> +                            UniqueVirtualMethod Overriding) {
> +  llvm::SmallVector<UniqueVirtualMethod, 4> &SubobjectOverrides
> +    = Overrides[OverriddenSubobject];
> +  if (std::find(SubobjectOverrides.begin(), SubobjectOverrides.end(), 
> +                Overriding) == SubobjectOverrides.end())
> +    SubobjectOverrides.push_back(Overriding);
> +}
> +
> +void OverridingMethods::add(const OverridingMethods &Other) {
> +  for (const_iterator I = Other.begin(), IE = Other.end(); I != IE; ++I) {
> +    for (overriding_const_iterator M = I->second.begin(), 
> +                                MEnd = I->second.end();
> +         M != MEnd; 
> +         ++M)
> +      add(I->first, *M);
> +  }
> +}
> +
> +void OverridingMethods::replaceAll(UniqueVirtualMethod Overriding) {
> +  for (iterator I = begin(), IEnd = end(); I != IEnd; ++I) {
> +    I->second.clear();
> +    I->second.push_back(Overriding);
> +  }
> +}
> +
> +
> +namespace {
> +  class FinalOverriderCollector {
> +    /// \brief The number of subobjects of a given class type that
> +    /// occur within the class hierarchy.
> +    llvm::DenseMap<const CXXRecordDecl *, unsigned> SubobjectCount;
> +
> +    /// \brief Overriders for each virtual base subobject.
> +    llvm::DenseMap<const CXXRecordDecl *, CXXFinalOverriderMap *> VirtualOverriders;
> +
> +    CXXFinalOverriderMap FinalOverriders;
> +
> +  public:
> +    ~FinalOverriderCollector();
> +
> +    void Collect(const CXXRecordDecl *RD, bool VirtualBase,
> +                 const CXXRecordDecl *InVirtualSubobject,
> +                 CXXFinalOverriderMap &Overriders);
> +  };
> +}
> +
> +void FinalOverriderCollector::Collect(const CXXRecordDecl *RD, 
> +                                      bool VirtualBase,
> +                                      const CXXRecordDecl *InVirtualSubobject,
> +                                      CXXFinalOverriderMap &Overriders) {
> +  unsigned SubobjectNumber = 0;
> +  if (!VirtualBase)
> +    SubobjectNumber
> +      = ++SubobjectCount[cast<CXXRecordDecl>(RD->getCanonicalDecl())];
> +
> +  for (CXXRecordDecl::base_class_const_iterator Base = RD->bases_begin(),
> +         BaseEnd = RD->bases_end(); Base != BaseEnd; ++Base) {
> +    if (const RecordType *RT = Base->getType()->getAs<RecordType>()) {
> +      const CXXRecordDecl *BaseDecl = cast<CXXRecordDecl>(RT->getDecl());
> +      if (!BaseDecl->isPolymorphic())
> +        continue;
> +
> +      if (Overriders.empty() && !Base->isVirtual()) {
> +        // There are no other overriders of virtual member functions,
> +        // so let the base class fill in our overriders for us.
> +        Collect(BaseDecl, false, InVirtualSubobject, Overriders);
> +        continue;
> +      }
> +
> +      // Collect all of the overridders from the base class subobject
> +      // and merge them into the set of overridders for this class.
> +      // For virtual base classes, populate or use the cached virtual
> +      // overrides so that we do not walk the virtual base class (and
> +      // its base classes) more than once.
> +      CXXFinalOverriderMap ComputedBaseOverriders;
> +      CXXFinalOverriderMap *BaseOverriders = &ComputedBaseOverriders;
> +      if (Base->isVirtual()) {
> +        CXXFinalOverriderMap *&MyVirtualOverriders = VirtualOverriders[BaseDecl];
> +        if (!MyVirtualOverriders) {
> +          MyVirtualOverriders = new CXXFinalOverriderMap;
> +          Collect(BaseDecl, true, BaseDecl, *MyVirtualOverriders);
> +        }
> +
> +        BaseOverriders = MyVirtualOverriders;
> +      } else
> +        Collect(BaseDecl, false, InVirtualSubobject, ComputedBaseOverriders);
> +
> +      // Merge the overriders from this base class into our own set of
> +      // overriders.
> +      for (CXXFinalOverriderMap::iterator OM = BaseOverriders->begin(), 
> +                               OMEnd = BaseOverriders->end();
> +           OM != OMEnd;
> +           ++OM) {
> +        const CXXMethodDecl *CanonOM
> +          = cast<CXXMethodDecl>(OM->first->getCanonicalDecl());
> +        Overriders[CanonOM].add(OM->second);
> +      }
> +    }
> +  }
> +
> +  for (CXXRecordDecl::method_iterator M = RD->method_begin(), 
> +                                   MEnd = RD->method_end();
> +       M != MEnd;
> +       ++M) {
> +    // We only care about virtual methods.
> +    if (!M->isVirtual())
> +      continue;
> +
> +    CXXMethodDecl *CanonM = cast<CXXMethodDecl>(M->getCanonicalDecl());
> +
> +    if (CanonM->begin_overridden_methods()
> +                                       == CanonM->end_overridden_methods()) {
> +      // This is a new virtual function that does not override any
> +      // other virtual function. Add it to the map of virtual
> +      // functions for which we are tracking overridders. 
> +
> +      // C++ [class.virtual]p2:
> +      //   For convenience we say that any virtual function overrides itself.
> +      Overriders[CanonM].add(SubobjectNumber,
> +                             UniqueVirtualMethod(CanonM, SubobjectNumber,
> +                                                 InVirtualSubobject));
> +      continue;
> +    }
> +
> +    // This virtual method overrides other virtual methods, so it does
> +    // not add any new slots into the set of overriders. Instead, we
> +    // replace entries in the set of overriders with the new
> +    // overrider. To do so, we dig down to the original virtual
> +    // functions using data recursion and update all of the methods it
> +    // overrides.
> +    typedef std::pair<CXXMethodDecl::method_iterator, 
> +                      CXXMethodDecl::method_iterator> OverriddenMethods;
> +    llvm::SmallVector<OverriddenMethods, 4> Stack;
> +    Stack.push_back(std::make_pair(CanonM->begin_overridden_methods(),
> +                                   CanonM->end_overridden_methods()));
> +    while (!Stack.empty()) {
> +      OverriddenMethods OverMethods = Stack.back();
> +      Stack.pop_back();
> +
> +      for (; OverMethods.first != OverMethods.second; ++OverMethods.first) {
> +        const CXXMethodDecl *CanonOM
> +          = cast<CXXMethodDecl>((*OverMethods.first)->getCanonicalDecl());
> +        if (CanonOM->begin_overridden_methods()
> +                                       == CanonOM->end_overridden_methods()) {
> +          // C++ [class.virtual]p2:
> +          //   A virtual member function C::vf of a class object S is
> +          //   a final overrider unless the most derived class (1.8)
> +          //   of which S is a base class subobject (if any) declares
> +          //   or inherits another member function that overrides vf.
> +          //
> +          // Treating this object like the most derived class, we
> +          // replace any overrides from base classes with this
> +          // overriding virtual function.
> +          Overriders[CanonOM].replaceAll(
> +                                 UniqueVirtualMethod(CanonM, SubobjectNumber,
> +                                                     InVirtualSubobject));
> +          continue;
> +        } 
> +
> +        // Continue recursion to the methods that this virtual method
> +        // overrides.
> +        Stack.push_back(std::make_pair(CanonOM->begin_overridden_methods(),
> +                                       CanonOM->end_overridden_methods()));
> +      }
> +    }
> +  }
> +}
> +
> +FinalOverriderCollector::~FinalOverriderCollector() {
> +  for (llvm::DenseMap<const CXXRecordDecl *, CXXFinalOverriderMap *>::iterator
> +         VO = VirtualOverriders.begin(), VOEnd = VirtualOverriders.end();
> +       VO != VOEnd; 
> +       ++VO)
> +    delete VO->second;
> +}
> +
> +void 
> +CXXRecordDecl::getFinalOverriders(CXXFinalOverriderMap &FinalOverriders) const {
> +  FinalOverriderCollector Collector;
> +  Collector.Collect(this, false, 0, FinalOverriders);
> +
> +  // Weed out any final overriders that come from virtual base class
> +  // subobjects that were hidden by other subobjects along any path.
> +  // This is the final-overrider variant of C++ [class.member.lookup]p10.
> +  for (CXXFinalOverriderMap::iterator OM = FinalOverriders.begin(), 
> +                           OMEnd = FinalOverriders.end();
> +       OM != OMEnd;
> +       ++OM) {
> +    for (OverridingMethods::iterator SO = OM->second.begin(), 
> +                                  SOEnd = OM->second.end();
> +         SO != SOEnd; 
> +         ++SO) {
> +      llvm::SmallVector<UniqueVirtualMethod, 4> &Overriding = SO->second;
> +      if (Overriding.size() < 2)
> +        continue;
> +
> +      for (llvm::SmallVector<UniqueVirtualMethod, 4>::iterator 
> +             Pos = Overriding.begin(), PosEnd = Overriding.end();
> +           Pos != PosEnd;
> +           /* increment in loop */) {
> +        if (!Pos->InVirtualSubobject) {
> +          ++Pos;
> +          continue;
> +        }
> +
> +        // We have an overriding method in a virtual base class
> +        // subobject (or non-virtual base class subobject thereof);
> +        // determine whether there exists an other overriding method
> +        // in a base class subobject that hides the virtual base class
> +        // subobject.
> +        bool Hidden = false;
> +        for (llvm::SmallVector<UniqueVirtualMethod, 4>::iterator
> +               OP = Overriding.begin(), OPEnd = Overriding.end();
> +             OP != OPEnd && !Hidden; 
> +             ++OP) {
> +          if (Pos == OP)
> +            continue;
> +
> +          if (OP->Method->getParent()->isVirtuallyDerivedFrom(
> +                         const_cast<CXXRecordDecl *>(Pos->InVirtualSubobject)))
> +            Hidden = true;
> +        }
> +
> +        if (Hidden) {
> +          // The current overriding function is hidden by another
> +          // overriding function; remove this one.
> +          Pos = Overriding.erase(Pos);
> +          PosEnd = Overriding.end();
> +        } else {
> +          ++Pos;
> +        }
> +      }
> +    }
> +  }
> +}
> 
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=99351&r1=99350&r2=99351&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Mar 23 18:47:56 2010
> @@ -1932,87 +1932,6 @@
>     SetBaseOrMemberInitializers(Constructor, 0, 0, false, false);
> }
> 
> -namespace {
> -  /// PureVirtualMethodCollector - traverses a class and its superclasses
> -  /// and determines if it has any pure virtual methods.
> -  class PureVirtualMethodCollector {
> -    ASTContext &Context;
> -
> -  public:
> -    typedef llvm::SmallVector<const CXXMethodDecl*, 8> MethodList;
> -
> -  private:
> -    MethodList Methods;
> -
> -    void Collect(const CXXRecordDecl* RD, MethodList& Methods);
> -
> -  public:
> -    PureVirtualMethodCollector(ASTContext &Ctx, const CXXRecordDecl* RD)
> -      : Context(Ctx) {
> -
> -      MethodList List;
> -      Collect(RD, List);
> -
> -      // Copy the temporary list to methods, and make sure to ignore any
> -      // null entries.
> -      for (size_t i = 0, e = List.size(); i != e; ++i) {
> -        if (List[i])
> -          Methods.push_back(List[i]);
> -      }
> -    }
> -
> -    bool empty() const { return Methods.empty(); }
> -
> -    MethodList::const_iterator methods_begin() { return Methods.begin(); }
> -    MethodList::const_iterator methods_end() { return Methods.end(); }
> -  };
> -
> -  void PureVirtualMethodCollector::Collect(const CXXRecordDecl* RD,
> -                                           MethodList& Methods) {
> -    // First, collect the pure virtual methods for the base classes.
> -    for (CXXRecordDecl::base_class_const_iterator Base = RD->bases_begin(),
> -         BaseEnd = RD->bases_end(); Base != BaseEnd; ++Base) {
> -      if (const RecordType *RT = Base->getType()->getAs<RecordType>()) {
> -        const CXXRecordDecl *BaseDecl = cast<CXXRecordDecl>(RT->getDecl());
> -        if (BaseDecl && BaseDecl->isAbstract())
> -          Collect(BaseDecl, Methods);
> -      }
> -    }
> -
> -    // Next, zero out any pure virtual methods that this class overrides.
> -    typedef llvm::SmallPtrSet<const CXXMethodDecl*, 4> MethodSetTy;
> -
> -    MethodSetTy OverriddenMethods;
> -    size_t MethodsSize = Methods.size();
> -
> -    for (RecordDecl::decl_iterator i = RD->decls_begin(), e = RD->decls_end();
> -         i != e; ++i) {
> -      // Traverse the record, looking for methods.
> -      if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(*i)) {
> -        // If the method is pure virtual, add it to the methods vector.
> -        if (MD->isPure())
> -          Methods.push_back(MD);
> -
> -        // Record all the overridden methods in our set.
> -        for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
> -             E = MD->end_overridden_methods(); I != E; ++I) {
> -          // Keep track of the overridden methods.
> -          OverriddenMethods.insert(*I);
> -        }
> -      }
> -    }
> -
> -    // Now go through the methods and zero out all the ones we know are
> -    // overridden.
> -    for (size_t i = 0, e = MethodsSize; i != e; ++i) {
> -      if (OverriddenMethods.count(Methods[i]))
> -        Methods[i] = 0;
> -    }
> -
> -  }
> -}
> -
> -
> bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
>                                   unsigned DiagID, AbstractDiagSelID SelID,
>                                   const CXXRecordDecl *CurrentRD) {
> @@ -2066,14 +1985,32 @@
>   if (PureVirtualClassDiagSet && PureVirtualClassDiagSet->count(RD))
>     return true;
> 
> -  PureVirtualMethodCollector Collector(Context, RD);
> +  CXXFinalOverriderMap FinalOverriders;
> +  RD->getFinalOverriders(FinalOverriders);
> 
> -  for (PureVirtualMethodCollector::MethodList::const_iterator I =
> -       Collector.methods_begin(), E = Collector.methods_end(); I != E; ++I) {
> -    const CXXMethodDecl *MD = *I;
> -
> -    Diag(MD->getLocation(), diag::note_pure_virtual_function) <<
> -      MD->getDeclName();
> +  for (CXXFinalOverriderMap::iterator M = FinalOverriders.begin(), 
> +                                   MEnd = FinalOverriders.end();
> +       M != MEnd; 
> +       ++M) {
> +    for (OverridingMethods::iterator SO = M->second.begin(), 
> +                                  SOEnd = M->second.end();
> +         SO != SOEnd; ++SO) {
> +      // C++ [class.abstract]p4:
> +      //   A class is abstract if it contains or inherits at least one
> +      //   pure virtual function for which the final overrider is pure
> +      //   virtual.
> +
> +      // 
> +      if (SO->second.size() != 1)
> +        continue;
> +
> +      if (!SO->second.front().Method->isPure())
> +        continue;
> +
> +      Diag(SO->second.front().Method->getLocation(), 
> +           diag::note_pure_virtual_function) 
> +        << SO->second.front().Method->getDeclName();
> +    }
>   }
> 
>   if (!PureVirtualClassDiagSet)
> @@ -2162,15 +2099,71 @@
>   for (UnresolvedSetIterator I = Convs->begin(), E = Convs->end(); I != E; ++I)
>     Convs->setAccess(I, (*I)->getAccess());
> 
> -  if (!Record->isAbstract()) {
> -    // Collect all the pure virtual methods and see if this is an abstract
> -    // class after all.
> -    PureVirtualMethodCollector Collector(Context, Record);
> -    if (!Collector.empty())
> -      Record->setAbstract(true);
> +  // Determine whether we need to check for final overriders. We do
> +  // this either when there are virtual base classes (in which case we
> +  // may end up finding multiple final overriders for a given virtual
> +  // function) or any of the base classes is abstract (in which case
> +  // we might detect that this class is abstract).
> +  bool CheckFinalOverriders = false;
> +  if (Record->isPolymorphic() && !Record->isInvalidDecl() &&
> +      !Record->isDependentType()) {
> +    if (Record->getNumVBases())
> +      CheckFinalOverriders = true;
> +    else if (!Record->isAbstract()) {
> +      for (CXXRecordDecl::base_class_const_iterator B = Record->bases_begin(),
> +                                                 BEnd = Record->bases_end();
> +           B != BEnd; ++B) {
> +        CXXRecordDecl *BaseDecl 
> +          = cast<CXXRecordDecl>(B->getType()->getAs<RecordType>()->getDecl());
> +        if (BaseDecl->isAbstract()) {
> +          CheckFinalOverriders = true; 
> +          break;
> +        }
> +      }
> +    }
> +  }
> +
> +  if (CheckFinalOverriders) {
> +    CXXFinalOverriderMap FinalOverriders;
> +    Record->getFinalOverriders(FinalOverriders);
> +
> +    for (CXXFinalOverriderMap::iterator M = FinalOverriders.begin(), 
> +                                     MEnd = FinalOverriders.end();
> +         M != MEnd; ++M) {
> +      for (OverridingMethods::iterator SO = M->second.begin(), 
> +                                    SOEnd = M->second.end();
> +           SO != SOEnd; ++SO) {
> +        assert(SO->second.size() > 0 && 
> +               "All virtual functions have overridding virtual functions");
> +        if (SO->second.size() == 1) {
> +          // C++ [class.abstract]p4:
> +          //   A class is abstract if it contains or inherits at least one
> +          //   pure virtual function for which the final overrider is pure
> +          //   virtual.
> +          if (SO->second.front().Method->isPure())
> +            Record->setAbstract(true);
> +          continue;
> +        }
> +
> +        // C++ [class.virtual]p2:
> +        //   In a derived class, if a virtual member function of a base
> +        //   class subobject has more than one final overrider the
> +        //   program is ill-formed.
> +        Diag(Record->getLocation(), diag::err_multiple_final_overriders)
> +          << (NamedDecl *)M->first << Record;
> +        Diag(M->first->getLocation(), diag::note_overridden_virtual_function);
> +        for (OverridingMethods::overriding_iterator OM = SO->second.begin(), 
> +                                                 OMEnd = SO->second.end();
> +             OM != OMEnd; ++OM)
> +          Diag(OM->Method->getLocation(), diag::note_final_overrider)
> +            << (NamedDecl *)M->first << OM->Method->getParent();
> +        
> +        Record->setInvalidDecl();
> +      }
> +    }
>   }
> 
> -  if (Record->isAbstract())
> +  if (Record->isAbstract() && !Record->isInvalidDecl())
>     (void)AbstractClassUsageDiagnoser(*this, Record);
> }
> 
> 
> Added: cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp?rev=99351&view=auto
> ==============================================================================
> --- cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp (added)
> +++ cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp Tue Mar 23 18:47:56 2010
> @@ -0,0 +1,80 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +namespace PR6631 {
> +  struct A { 
> +    virtual void f() = 0;
> +  };
> +
> +  struct B : virtual A { };
> +
> +  struct C : virtual A { 
> +    virtual void f();
> +  };
> +
> +  struct D : public B, public C { 
> +    virtual void f();
> +  };
> +
> +  void f() {
> +    (void)new D; // okay
> +  }
> +}
> +
> +// Check cases where we have a virtual function that is pure in one
> +// subobject but not pure in another subobject.
> +namespace PartlyPure {
> +  struct A { 
> +    virtual void f() = 0; // expected-note{{pure virtual function}}
> +  };
> +
> +  struct B : A {
> +    virtual void f();
> +  };
> +
> +  struct C : virtual A { };
> +
> +  struct D : B, C { };
> +
> +  void f() {
> +    (void) new D; // expected-error{{abstract type}}
> +  }
> +}
> +
> +namespace NonPureAlongOnePath {
> +  struct A { 
> +    virtual void f() = 0;
> +  };
> +
> +  struct B : virtual A {
> +    virtual void f();
> +  };
> +
> +  struct C : virtual A { };
> +
> +  struct D : B, C { };
> +
> +  void f() {
> +    (void) new D; // okay
> +  }  
> +}
> +
> +namespace NonPureAlongOnePath2 {
> +  struct Aprime { 
> +    virtual void f() = 0;
> +  };
> +
> +  struct A : Aprime {
> +  };
> +
> +  struct B : virtual A {
> +    virtual void f();
> +  };
> +
> +  struct C : virtual A { };
> +
> +  struct D : B, C { };
> +
> +  void f() {
> +    (void) new D; // okay
> +  }  
> +}
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp
> ------------------------------------------------------------------------------
>    svn:eol-style = native
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp
> ------------------------------------------------------------------------------
>    svn:keywords = Id
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p4.cpp
> ------------------------------------------------------------------------------
>    svn:mime-type = text/plain
> 
> Added: cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp?rev=99351&view=auto
> ==============================================================================
> --- cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp (added)
> +++ cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp Tue Mar 23 18:47:56 2010
> @@ -0,0 +1,23 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +struct A {
> +  virtual void f() = 0; // expected-note{{pure virtual function}}
> +};
> +
> +struct B : A {
> +  virtual void f();
> +};
> +
> +struct C : B {
> +  virtual void f() = 0; // expected-note 2{{pure virtual function}}
> +};
> +
> +struct D : C {
> +};
> +
> +void test() {
> +  (void)new A; // expected-error{{object of abstract type}}
> +  (void)new B;
> +  (void)new C; // expected-error{{object of abstract type}}
> +  (void)new D; // expected-error{{object of abstract type}}
> +}
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp
> ------------------------------------------------------------------------------
>    svn:eol-style = native
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp
> ------------------------------------------------------------------------------
>    svn:keywords = Id
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.abstract/p5.cpp
> ------------------------------------------------------------------------------
>    svn:mime-type = text/plain
> 
> Added: cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp?rev=99351&view=auto
> ==============================================================================
> --- cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp (added)
> +++ cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp Tue Mar 23 18:47:56 2010
> @@ -0,0 +1,37 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +struct A {
> +  virtual void f() = 0; // expected-note 2{{overridden virtual function}}
> +};
> +
> +struct Aprime : virtual A {
> +  virtual void f();
> +};
> +
> +struct B : Aprime {
> +  virtual void f(); // expected-note 3{{final overrider of 'A::f'}}
> +};
> +
> +struct C : virtual A {
> +  virtual void f(); // expected-note{{final overrider of 'A::f'}}
> +};
> +
> +struct D : B, C { }; // expected-error{{virtual function 'A::f' has more than one final overrider in 'D'}}
> +
> +struct B2 : B { };
> +
> +struct E : B, B2 { }; //expected-error{{virtual function 'A::f' has more than one final overrider in 'E'}}
> +
> +struct F : B, B2 {
> +  virtual void f(); // okay
> +};
> +
> +struct G : F { }; // okay
> +
> +struct H : G, A { }; // okay
> +
> +namespace MultipleSubobjects {
> +  struct A { virtual void f(); };
> +  struct B : A { virtual void f(); };
> +  struct C : A { virtual void f(); };
> +  struct D : B, C { }; // okay
> +}
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp
> ------------------------------------------------------------------------------
>    svn:eol-style = native
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp
> ------------------------------------------------------------------------------
>    svn:keywords = Id
> 
> Propchange: cfe/trunk/test/CXX/class.derived/class.virtual/p2.cpp
> ------------------------------------------------------------------------------
>    svn:mime-type = text/plain
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list