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