[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
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
----------------
kadircet wrote:
> hokein wrote:
> > `Current` seems not clear enough, how about "MatchNamespace"?
> what about enclosing?
sounds good.


================
Comment at: clang-tools-extra/clangd/SourceCode.h:273
+  /// definition.
+  std::vector<Position> EligiblePoints;
+};
----------------
kadircet wrote:
> hokein wrote:
> > 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()`).
> for example a caller with access to AST and Index, could first look at the decls surrounding the target(fully qualified name), then check for their definition locations using index and find an eligible point between these two.
Fair enough, thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024