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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
