[clang] [Clang][AST] Fix a crash on attaching doc comments (PR #78716)

via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 21 19:48:03 PST 2024


chenshanzhi wrote:

@gribozavr  
Thanks for your reply! I'm really glad that I can get the explanation about the original aim of the relevant code from you!

I do have considered merging the two loops as you recommended when I tried to solve the crash. But I got confused when I compared the implementation of `ASTContext::attachCommentsToJustParsedDecls` and `ASTContext::getRawCommentForDeclNoCache`. And I'm not sure whether what we want for `ASTContext::attachCommentsToJustParsedDecls` is just like a `ASTContext::getRawCommentForDeclNoCache` with additional cache operations after calling `getRawCommentForDeclNoCacheImpl`? 
Actually, when I tried to solve the crash and glanced over the refactor changes in f31d8df1c8c69e7a787c1c1c529a524f3001c66a, I thought the aim of introducing `attachCommentsToJustParsedDecls` might be achieving some level of speed-ups. And the comment `// The location doesn't have to be precise - we care only about the file.` made me think some of the speed-ups might come from the early `break` in the first loop and the author might be inlined to use two loops and an early break in the first loop. That's why I did not choose to merge the two loops in this crash fix commit. I totally agree that merging the two loops would make this code more robust. I think we could merge this crash fix change first and open another issue to track further improvements.

https://github.com/llvm/llvm-project/pull/78716


More information about the cfe-commits mailing list