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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to