[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.


================
Comment at: include/clang/AST/DeclarationName.h:243
+  ///   C++ literal operator, or C++ using directive.
   uintptr_t Ptr = 0;
 
----------------
erichkeane wrote:
> There is an llvm type for storing something in the lower bits of a pointer.  I THINK it is llvm::PointerIntPair.
> 
> I'd MUCH prefer you do that instead of the reimplementation here.
Well it was already like this but point taken.


================
Comment at: lib/AST/DeclarationName.cpp:42
 
+void DeclarationName::VerifyStaticAsserts() {
+  llvm_unreachable("VerifyStaticAsserts is not meant to be called!");
----------------
erichkeane wrote:
> I'm a touch confused by this function.  It shouldn't be necessary when the static asserts can all be put at the global/class level.
Yes, but the enumeration StoredNameKind is private to DeclarationName.
Therefore I cannot just put the static_asserts into a .cpp and be done with it.
An alternative is of course to put them in the class definition in the header but
look at this mess... (and this is after clang-format).


Repository:
  rC Clang

https://reviews.llvm.org/D52267