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

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 19 08:54:17 PDT 2018


erichkeane added a comment.

A few drive-by comments... This is a patch with interactions I'm not sure I feel comfortable approving myself, so I'll count on @rjmccall to do so.



================
Comment at: include/clang/AST/DeclarationName.h:243
+  ///   C++ literal operator, or C++ using directive.
   uintptr_t Ptr = 0;
 
----------------
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.


================
Comment at: lib/AST/DeclarationName.cpp:42
 
+void DeclarationName::VerifyStaticAsserts() {
+  llvm_unreachable("VerifyStaticAsserts is not meant to be called!");
----------------
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.


Repository:
  rC Clang

https://reviews.llvm.org/D52267





More information about the cfe-commits mailing list