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

[Jan Korous via Phabricator via cfe-commits]

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();
 
----------------
gribozavr wrote:
> jkorous wrote:
> > 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?
> > 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).
> 
> Only if there was only one comment in the redeclaration chain.  If a redeclaration had a different comment, this function would return that comment.
I see. I made a wrong assumption - that for any two redeclarations the redeclaration chain always starts from the same declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432