[clang] [clang-tools-extra] [clangd] Collect comments from function definitions into the index (PR #67802)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Sun Sep 8 23:52:01 PDT 2024


https://github.com/HighCommander4 requested changes to this pull request.

Apologies for the slow response time. I finally had a chance to look at this in a bit more detail, though I still don't think I fully understand this patch.

At this point, my main feedback is that it would be good to include some unit tests. `SymbolCollectorTests.cpp` is a good place for them; see e.g. [this test](https://searchfox.org/llvm/rev/78fa41524b6f6e2696ff21ec50e760311ac939a3/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp#413) for an example of testing on a header/source file pair as input, and [this test](https://searchfox.org/llvm/rev/78fa41524b6f6e2696ff21ec50e760311ac939a3/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp#1455) for an example of testing the value of the documentation field.

What is the purpose of the change to `ASTContext.cpp`? This codepath has consumers other than clangd, so if we're changing the behaviour, we need to evaluate whether it's appropriate for other consumers (and maybe add libAST unit tests as well).

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


More information about the cfe-commits mailing list