https://github.com/HighCommander4 requested changes to this pull request.
Thanks, this patch generally looks great. It would be nice to have a couple of test cases exercising the behaviour with overloaded methods when overload bundling is enabled (`CodeCompleteOptions::BundleOverloads` is set to true): * if we have an overload set where multiple methods have documentation, we pick an unspecified one of them ([here](https://searchfox.org/llvm/rev/e8f1902cca2aeaa23db6080fe33fbc67ed203350/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#2005) is an existing assertion for that, in a test case exercising the non-header scenario) * if we have an overload set where some but not all methods have documentation, we pick one that does (rather than one that doesn't) I think the code already does the right thing and should pass these testcases, but it's good to verify and have test coverage for them. (The template issue discussed in the previous two comments can be left to a follow-up PR to investigate and improve on.) https://github.com/llvm/llvm-project/pull/120099 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
