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

Fri Dec 13 14:41:48 PST 2019

aprantl added inline comments.

================
Comment at: clang/include/clang/AST/ExternalASTSource.h:69

-  /// Whether this AST source also provides information for
-  /// semantic analysis.
-  bool SemaSource = false;
+  // LLVM-style RTTI.
+  static char ID;
----------------
rjmccall wrote:
> aprantl wrote:
> > /// LLVM-style RTTI.
> 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.
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.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71397/new/

https://reviews.llvm.org/D71397