[clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

[teemperor]

Summary

We currently have some very basic LLVM-style RTTI support in the ExternalASTSource class hierarchy
based on the SemaSource bool( to discriminate it form the ExternalSemaSource). As ExternalASTSource
is supposed to be subclassed we should have extendable LLVM-style RTTI in this class hierarchy to make life easier
for projects building on top of Clang.

Most notably the current RTTI implementation forces LLDB to implement RTTI for its
own ExternalASTSource class (ClangExternalASTSourceCommon) by keeping a global set of
ExternalASTSources that are known to be ClangExternalASTSourceCommon. Projects
using Clang currently have to dosimilar workarounds to get RTTI support for their subclasses.

This patch turns this into full-fledged LLVM-style RTTI based on a static ID variable similar to
other LLVM class hierarchies. Also removes the friend declaration from ExternalASTSource to
its child class that was only used to grant access to the SemaSource member.

[teemperor]

I didn't add specific ID's to all of the subclasses in clang/clang-tools-extra because we never actually check for these specific classes anywhere from what I can see. But if anyone thinks that would be useful to have then I can update the patch.

[teemperor]

See D71398 for the LLDB removal of our own RTTI workaround.

[aprantl]

clang/include/clang/AST/ExternalASTSource.h

69–70

/// LLVM-style RTTI.

327

/// LLVM-style RTTI.
/// \{
...
/// \}

clang/include/clang/Sema/ExternalSemaSource.h

53

etc

[lhames]

Side note: This https://reviews.llvm.org/D39111 seems related. The patch has languished as I have been busy with other work, but if it would be useful for you guys I'd be happy to try to land it.

[rjmccall]

clang/include/clang/AST/ExternalASTSource.h

69–70

It would be better if this had a dedicated type instead of using char and then void* as the parameter, just to prevent simple bugs where somebody passes in the wrong pointer. Maybe you could have a non-copyable ClassID type and a ClassRef type with the sole constructor ClassRef(const ClassID &)?

The virtual method name is trying too hard to be succinct, and it doesn't matter because it's rarely used; I would recommend borrowing the name isKindOf from Objective-C.

[aprantl]

clang/include/clang/AST/ExternalASTSource.h

69–70

This is a pattern that is used all over in LLDB. However, these are good suggestions, and we might want to make those improvements there as well.

[rjmccall]

If this is commonly used, it would make sense to introduce those types into some common header in LLVM.

[teemperor]

In D71397#1782537, @lhames wrote:

Side note: This https://reviews.llvm.org/D39111 seems related. The patch has languished as I have been busy with other work, but if it would be useful for you guys I'd be happy to try to land it.

Moving ExternalASTSource to that system would also work, but that would add yet another dependency to D71398 (which is removing really gnarly code). But otherwise this seems like something we could use here (and also in LLDB).

I would prefer landing this as-is because it is just doing the same we do in other places in LLVM. Afterwards migrating the whole RTTI system in LLVM to a new system (whether that will be D39111 or just a ClassID type) sounds good to me.

[rjmccall]

If we decide as a community that D39111 is better, we can go back and adopt it at any point; I don't see that as a reason to reject modest refinements, especially given that there is no sign whatsoever of movement on D39111.

[lhames]

In D71397#1784325, @teemperor wrote:

I would prefer landing this as-is because it is just doing the same we do in other places in LLVM. Afterwards migrating the whole RTTI system in LLVM to a new system (whether that will be D39111 or just a ClassID type) sounds good to me.

Yep -- To be clear I didn't raise D39111 as an objection to this approach, just as a matter of interest. As you noted it should be easy to switch later.

[teemperor]

• Addressed Adrian's comments.

clang/include/clang/AST/ExternalASTSource.h

 /// contexts.
 class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
   friend class ExternalSemaSource;
 
   /// Generation number for this external AST source. Must be increased
   /// whenever we might have added new redeclarations for existing decls.
   uint32_t CurrentGeneration = 0;
 
-  /// Whether this AST source also provides information for
-  /// semantic analysis.
-  bool SemaSource = false;
+  /// LLVM-style RTTI.
+  static char ID;
 
 public:
   ExternalASTSource() = default;
   virtual ~ExternalASTSource();
 
   /// RAII class for safely pairing a StartedDeserializing call
   /// with FinishedDeserializing.
   class Deserializing {
public:
   MemoryBufferSizes getMemoryBufferSizes() const {
     MemoryBufferSizes sizes(0, 0);
     getMemoryBufferSizes(sizes);
     return sizes;
   }
 
   virtual void getMemoryBufferSizes(MemoryBufferSizes &sizes) const;
 
+  /// LLVM-style RTTI.
+  /// \{
+  virtual bool isA(const void *ClassID) const { return ClassID == &ID; }
+  static bool classof(const ExternalASTSource *S) { return S->isA(&ID); }
+  /// \}
+
 protected:
   static DeclContextLookupResult
   SetExternalVisibleDeclsForName(const DeclContext *DC,
                                  DeclarationName Name,
                                  ArrayRef<NamedDecl*> Decls);
 
   static DeclContextLookupResult
   SetNoExternalVisibleDeclsForName(const DeclContext *DC,

clang/include/clang/Sema/ExternalSemaSource.h

struct ExternalVTableUse {
   SourceLocation Location;
   bool DefinitionRequired;
 };
 
 /// An abstract interface that should be implemented by
 /// external AST sources that also provide information for semantic
 /// analysis.
 class ExternalSemaSource : public ExternalASTSource {
+  /// LLVM-style RTTI.
+  static char ID;
+
 public:
-  ExternalSemaSource() {
-    ExternalASTSource::SemaSource = true;
-  }
+  ExternalSemaSource() = default;
 
   ~ExternalSemaSource() override;
 
   /// Initialize the semantic source with the Sema instance
   /// being used to perform semantic analysis on the abstract syntax
   /// tree.
   virtual void InitializeSema(Sema &S) {}
 
public:
   /// \param T the \c QualType that should have been complete at \p Loc
   ///
   /// \return true if a diagnostic was produced, false otherwise.
   virtual bool MaybeDiagnoseMissingCompleteType(SourceLocation Loc,
                                                 QualType T) {
     return false;
   }
 
-  // isa/cast/dyn_cast support
-  static bool classof(const ExternalASTSource *Source) {
-    return Source->SemaSource;
+  /// LLVM-style RTTI.
+  /// \{
+  bool isA(const void *ClassID) const override {
+    return ClassID == &ID || ExternalASTSource::isA(ClassID);
   }
+  static bool classof(const ExternalASTSource *S) { return S->isA(&ID); }
+  /// \}
 };
 
 } // end namespace clang
 
 #endif

clang/include/clang/Sema/MultiplexExternalSemaSource.h

namespace clang {
   class ValueDecl;
   class VarDecl;
 
 
 /// An abstract interface that should be implemented by
 /// external AST sources that also provide information for semantic
 /// analysis.
 class MultiplexExternalSemaSource : public ExternalSemaSource {
+  /// LLVM-style RTTI.
+  static char ID;
 
 private:
   SmallVector<ExternalSemaSource *, 2> Sources; // doesn't own them.
 
 public:
 
   ///Constructs a new multiplexing external sema source and appends the
   /// given element to it.
public:
   /// provided
   ///
   /// \param T the \c QualType that should have been complete at \p Loc
   ///
   /// \return true if a diagnostic was produced, false otherwise.
   bool MaybeDiagnoseMissingCompleteType(SourceLocation Loc,
                                         QualType T) override;
 
-  // isa/cast/dyn_cast support
-  static bool classof(const MultiplexExternalSemaSource*) { return true; }
-  //static bool classof(const ExternalSemaSource*) { return true; }
+  /// LLVM-style RTTI.
+  /// \{
+  bool isA(const void *ClassID) const override {
+    return ClassID == &ID || ExternalSemaSource::isA(ClassID);
+  }
+  static bool classof(const ExternalASTSource *S) { return S->isA(&ID); }
+  /// \}
 };
 
 } // end namespace clang
 
 #endif

clang/lib/AST/ExternalASTSource.cpp

 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Module.h"
 #include "llvm/ADT/None.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstdint>
 
 using namespace clang;
 
+char ExternalASTSource::ID;
+
 ExternalASTSource::~ExternalASTSource() = default;
 
 llvm::Optional<ExternalASTSource::ASTSourceDescriptor>
 ExternalASTSource::getSourceDescriptor(unsigned ID) {
   return None;
 }
 
 ExternalASTSource::ExtKind

clang/lib/Sema/MultiplexExternalSemaSource.cpp

 //
 //===----------------------------------------------------------------------===//
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/AST/DeclContextInternals.h"
 #include "clang/Sema/Lookup.h"
 
 using namespace clang;
 
+char MultiplexExternalSemaSource::ID;
+
 ///Constructs a new multiplexing external sema source and appends the
 /// given element to it.
 ///
 MultiplexExternalSemaSource::MultiplexExternalSemaSource(ExternalSemaSource &s1,
                                                         ExternalSemaSource &s2){
   Sources.push_back(&s1);
   Sources.push_back(&s2);
 }

clang/lib/Sema/Sema.cpp

if (RC.isAlmostTrailingComment()) {
     Diag(Comment.getBegin(), diag::warn_not_a_doxygen_trailing_member_comment) <<
       FixItHint::CreateReplacement(MagicMarkerRange, MagicMarkerText);
   }
   Context.addComment(RC);
 }
 
 // Pin this vtable to this file.
 ExternalSemaSource::~ExternalSemaSource() {}
+char ExternalSemaSource::ID;
 
 void ExternalSemaSource::ReadMethodPool(Selector Sel) { }
 void ExternalSemaSource::updateOutOfDateSelector(Selector Sel) { }
 
 void ExternalSemaSource::ReadKnownNamespaces(
                            SmallVectorImpl<NamespaceDecl *> &Namespaces) {
 }