[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

Jan Korous via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 18:29:12 PDT 2019


jkorous marked an inline comment as done.
jkorous added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 
----------------
arphaman wrote:
> Why are we now checking for the canonical declaration instead of `D` as before?
Before this we were caching comment for every redeclaration separately but for every redeclaration the comment was the same.

As I understand it - for a given declaration we found the first comment in the redeclaration chain and then saved it to the cache for every redeclaration (the same comment).
Later, during lookup, we iterated the redeclaration chain again and did a lookup for every redeclaration (see L392 in the original implementation).

Unless I missed something, it's suboptimal in both memory consumption and runtime overhead.

That's the reason why I want to cache the comment for the redeclaration group as a whole. The only thing I am not entirely sure about is what key to use to represent the whole redeclaration chain - maybe the first declaration in the redeclaration chain would be better then the canonical declaration...

WDYT?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432





More information about the cfe-commits mailing list