[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The implementation looks good, just have some questions on the API.



================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:997
+  // definitions as well. One might use a closing parantheses(")" followed by an
+  // opening brace "{" to trigger the start.
+  parseNamespaceEvents(Code, Style, [&](NamespaceEvent Event) {
----------------
If I understand the FIXME well, so we will extend the API to parse functions, so that we could get a more eligible point e.g. near the near the neighboring declaration.


================
Comment at: clang-tools-extra/clangd/SourceCode.h:270
+  /// It will be “a::b” for both carrot locations.
+  std::string CurrentNamespace;
+  /// Offsets into the code marking eligible points to insert a function
----------------
`Current` seems not clear enough, how about "MatchNamespace"?


================
Comment at: clang-tools-extra/clangd/SourceCode.h:273
+  /// definition.
+  std::vector<Position> EligiblePoints;
+};
----------------
I'm curious how the caller use this, it seems that the caller has no/little information to distinguish all the positions, even for a simple case where we add a definition at the last eligible point (I assume just just using `EligiblePoints.back()`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68024/new/

https://reviews.llvm.org/D68024