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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/AST/ASTContext.h:756
   /// lazily.
   mutable llvm::DenseMap<const Decl *, RawCommentAndCacheFlags> RedeclComments;
 
----------------
The name of this variable (and `RawCommentAndCacheFlags`) don't make sense after the change.


================
Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 
----------------
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432