Clean up and document the representation of C++ base classes

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@58040 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 81 insertions, 52 deletions

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index ccafaae317..e9d159ee76 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -40,10 +40,29 @@ public:
   static bool classof(const CXXFieldDecl *D) { return true; }
 };
 
-/// BaseSpecifier - A base class of a C++ class.
+/// CXXBaseSpecifier - A base class of a C++ class.
+///
+/// Each CXXBaseSpecifier represents a single, direct base class (or
+/// struct) of a C++ class (or struct). It specifies the type of that
+/// base class, whether it is a virtual or non-virtual base, and what
+/// level of access (public, protected, private) is used for the
+/// derivation. For example:
+///
+/// @code
+///   class A { };
+///   class B { };
+///   class C : public virtual A, protected B { };
+/// @endcode
+///
+/// In this code, C will have two CXXBaseSpecifiers, one for "public
+/// virtual A" and the other for "protected B".
 class CXXBaseSpecifier {
+  /// Range - The source code range that covers the full base
+  /// specifier, including the "virtual" (if present) and access
+  /// specifier (if present).
   SourceRange Range;
 
+  /// Virtual - Whether this is a virtual base class or not.
   bool Virtual : 1;
 
   /// BaseOfClass - Whether this is the base of a class (true) or of a
@@ -52,21 +71,22 @@ class CXXBaseSpecifier {
   /// used for semantic analysis.
   bool BaseOfClass : 1; 
 
-  /// Access specifier as written in the source code (which may be
-  /// AS_none). The actual type of data stored here is an
+  /// Access - Access specifier as written in the source code (which
+  /// may be AS_none). The actual type of data stored here is an
   /// AccessSpecifier, but we use "unsigned" here to work around a
   /// VC++ bug.
   unsigned Access : 2;
 
+  /// BaseType - The type of the base class. This will be a class or
+  /// struct (or a typedef of such).
   QualType BaseType;
   
+public:
+  CXXBaseSpecifier() { }
+
   CXXBaseSpecifier(SourceRange R, bool V, bool BC, AccessSpecifier A, QualType T)
     : Range(R), Virtual(V), BaseOfClass(BC), Access(A), BaseType(T) { }
 
-public:
-  static CXXBaseSpecifier *Create(ASTContext &C, SourceRange R, bool V, bool BC, 
-                                  AccessSpecifier A, QualType T);
-
   /// getSourceRange - Retrieves the source range that contains the
   /// entire base specifier.
   SourceRange getSourceRange() const { return Range; }
@@ -108,7 +128,7 @@ public:
 class CXXRecordDecl : public RecordDecl, public DeclContext {
   /// Bases - Base classes of this class.
   /// FIXME: This is wasted space for a union.
-  CXXBaseSpecifier **Bases;
+  CXXBaseSpecifier *Bases;
 
   /// NumBases - The number of base class specifiers in Bases.
   unsigned NumBases;
@@ -121,31 +141,30 @@ class CXXRecordDecl : public RecordDecl, public DeclContext {
   ~CXXRecordDecl();
 
 public:
+  /// base_class_iterator - Iterator that traverses the base classes
+  /// of a clas.
+  typedef CXXBaseSpecifier*       base_class_iterator;
+
+  /// base_class_const_iterator - Iterator that traverses the base
+  /// classes of a clas.
+  typedef const CXXBaseSpecifier* base_class_const_iterator;
+
   static CXXRecordDecl *Create(ASTContext &C, TagKind TK, DeclContext *DC,
                                SourceLocation L, IdentifierInfo *Id,
                                CXXRecordDecl* PrevDecl=0);
   
   /// setBases - Sets the base classes of this struct or class.
-  void setBases(CXXBaseSpecifier **Bases, unsigned NumBases) {
-    this->Bases = Bases;
-    this->NumBases = NumBases;
-  }
+  void setBases(CXXBaseSpecifier const * const *Bases, unsigned NumBases);
 
   /// getNumBases - Retrieves the number of base classes of this
   /// class.
   unsigned getNumBases() const { return NumBases; }
 
-  /// getBase - Retrieve the ith base class.
-  CXXBaseSpecifier *getBase(unsigned i) { 
-    assert(i < NumBases && "Base index out of range");
-    return Bases[i];
-  }
+  base_class_iterator       bases_begin()       { return Bases; }
+  base_class_const_iterator bases_begin() const { return Bases; }
+  base_class_iterator       bases_end()         { return Bases + NumBases; }
+  base_class_const_iterator bases_end()   const { return Bases + NumBases; }
 
-  /// getBase - Retrieve the ith base class.
-  const CXXBaseSpecifier *getBase(unsigned i) const { 
-    assert(i < NumBases && "Base index out of range");
-    return Bases[i];
-  }
 
   const CXXFieldDecl *getMember(unsigned i) const {
     return cast<const CXXFieldDecl>(RecordDecl::getMember(i));
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 86db8532c8..8baf4196bf 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -26,14 +26,6 @@ CXXFieldDecl *CXXFieldDecl::Create(ASTContext &C, CXXRecordDecl *RD,
   return new (Mem) CXXFieldDecl(RD, L, Id, T, BW);
 }
 
-CXXBaseSpecifier *CXXBaseSpecifier::Create(ASTContext &C, SourceRange R, bool V, 
-                                           bool BC, AccessSpecifier A, QualType T)
-{
-  void *Mem = C.getAllocator().Allocate<CXXBaseSpecifier>();
-  CXXBaseSpecifier* BS = new (Mem) CXXBaseSpecifier(R, V, BC, A, T);
-  return BS;
-}
-
 CXXRecordDecl *CXXRecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
                                      SourceLocation L, IdentifierInfo *Id,
                                      CXXRecordDecl* PrevDecl) {
@@ -44,11 +36,21 @@ CXXRecordDecl *CXXRecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
 }
 
 CXXRecordDecl::~CXXRecordDecl() {
-  for (unsigned i = 0; i < NumBases; ++i)
-    delete Bases[i];
   delete [] Bases;
 }
 
+void 
+CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, 
+                        unsigned NumBases) {
+  if (this->Bases)
+    delete [] this->Bases;
+
+  this->Bases = new CXXBaseSpecifier[NumBases];
+  this->NumBases = NumBases;
+  for (unsigned i = 0; i < NumBases; ++i)
+    this->Bases[i] = *Bases[i];
+}
+
 CXXMethodDecl *
 CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD,
                       SourceLocation L, IdentifierInfo *Id,
diff --git a/lib/AST/InheritViz.cpp b/lib/AST/InheritViz.cpp
index b817ed613f..bfd859f527 100644
--- a/lib/AST/InheritViz.cpp
+++ b/lib/AST/InheritViz.cpp
@@ -92,8 +92,8 @@ void InheritanceHierarchyWriter::WriteNode(QualType Type, bool FromVirtual) {
   // Display the base classes.
   const CXXRecordDecl *Decl 
     = static_cast<const CXXRecordDecl *>(Type->getAsRecordType()->getDecl());
-  for (unsigned idx = 0; idx < Decl->getNumBases(); ++idx) {
-    const CXXBaseSpecifier *Base = Decl->getBase(idx);
+  for (CXXRecordDecl::base_class_const_iterator Base = Decl->bases_begin();
+       Base != Decl->bases_end(); ++Base) {
     QualType CanonBaseType = Context.getCanonicalType(Base->getType());
 
     // If this is not virtual inheritance, bump the direct base
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index f17875b986..729bd7c35c 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -299,11 +299,8 @@ Sema::ActOnBaseSpecifier(DeclTy *classdecl, SourceRange SpecifierRange,
   }
 
   // Create the base specifier.
-  CXXBaseSpecifier *BS = CXXBaseSpecifier::Create(Context, SpecifierRange, 
-                                                  Virtual, 
-                                                  BaseType->isClassType(),
-                                                  Access, BaseType);
-  return BS;
+  return new CXXBaseSpecifier(SpecifierRange, Virtual, 
+                              BaseType->isClassType(), Access, BaseType);
 }
 
 /// ActOnBaseSpecifiers - Attach the given base specifiers to the
@@ -316,34 +313,45 @@ void Sema::ActOnBaseSpecifiers(DeclTy *ClassDecl, BaseTy **Bases,
 
   // Used to keep track of which base types we have already seen, so
   // that we can properly diagnose redundant direct base types. Note
-  // that the key is always the canonical type.
+  // that the key is always the unqualified canonical type of the base
+  // class.
   std::map<QualType, CXXBaseSpecifier*, QualTypeOrdering> KnownBaseTypes;
 
   // Copy non-redundant base specifiers into permanent storage.
-  CXXBaseSpecifier **InBaseSpecs = (CXXBaseSpecifier **)Bases;
-  CXXBaseSpecifier **StoredBaseSpecs = new CXXBaseSpecifier* [NumBases];
-  unsigned outIdx = 0;
-  for (unsigned inIdx = 0; inIdx < NumBases; ++inIdx) {
+  CXXBaseSpecifier **BaseSpecs = (CXXBaseSpecifier **)Bases;
+  unsigned NumGoodBases = 0;
+  for (unsigned idx = 0; idx < NumBases; ++idx) {
     QualType NewBaseType 
-      = Context.getCanonicalType(InBaseSpecs[inIdx]->getType());
+      = Context.getCanonicalType(BaseSpecs[idx]->getType());
+    NewBaseType = NewBaseType.getUnqualifiedType();
+
     if (KnownBaseTypes[NewBaseType]) {
       // C++ [class.mi]p3:
       //   A class shall not be specified as a direct base class of a
       //   derived class more than once.
-      Diag(InBaseSpecs[inIdx]->getSourceRange().getBegin(),
+      Diag(BaseSpecs[idx]->getSourceRange().getBegin(),
            diag::err_duplicate_base_class, 
            KnownBaseTypes[NewBaseType]->getType().getAsString(),
-           InBaseSpecs[inIdx]->getSourceRange());
+           BaseSpecs[idx]->getSourceRange());
+
+      // Delete the duplicate base class specifier; we're going to
+      // overwrite its pointer later.
+      delete BaseSpecs[idx];
     } else {
       // Okay, add this new base class.
-      KnownBaseTypes[NewBaseType] = InBaseSpecs[inIdx];
-      StoredBaseSpecs[outIdx++] = InBaseSpecs[inIdx];
+      KnownBaseTypes[NewBaseType] = BaseSpecs[idx];
+      BaseSpecs[NumGoodBases++] = BaseSpecs[idx];
     }
   }
 
   // Attach the remaining base class specifiers to the derived class.
   CXXRecordDecl *Decl = (CXXRecordDecl*)ClassDecl;
-  Decl->setBases(StoredBaseSpecs, outIdx);
+  Decl->setBases(BaseSpecs, NumGoodBases);
+
+  // Delete the remaining (good) base class specifiers, since their
+  // data has been copied into the CXXRecordDecl.
+  for (unsigned idx = 0; idx < NumGoodBases; ++idx)
+    delete BaseSpecs[idx];
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/lib/Sema/SemaInherit.cpp b/lib/Sema/SemaInherit.cpp
index 022c8043d0..6890dbfc64 100644
--- a/lib/Sema/SemaInherit.cpp
+++ b/lib/Sema/SemaInherit.cpp
@@ -39,8 +39,8 @@ bool Sema::IsDerivedFrom(QualType Derived, QualType Base)
   if (const RecordType *DerivedType = Derived->getAsRecordType()) {
     const CXXRecordDecl *Decl 
       = static_cast<const CXXRecordDecl *>(DerivedType->getDecl());
-    for (unsigned idx = 0; idx < Decl->getNumBases(); ++idx) {
-      const CXXBaseSpecifier *BaseSpec = Decl->getBase(idx);
+    for (CXXRecordDecl::base_class_const_iterator BaseSpec = Decl->bases_begin();
+         BaseSpec != Decl->bases_end(); ++BaseSpec) {
       if (Context.getCanonicalType(BaseSpec->getType()) == Base
           || IsDerivedFrom(BaseSpec->getType(), Base))
         return true;