[PATCH] D53131: [clangd] Support scope proximity in code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Awesome! Just nits.



================
Comment at: clangd/CodeComplete.cpp:1224
   std::vector<std::string> QueryScopes; // Initialized once Sema runs.
+  // Initialized once QueryScopes is initialized.
+  llvm::Optional<ScopeDistance> ScopeProximity;
----------------
, if there are scopes?


================
Comment at: clangd/FileDistance.cpp:206
+    // The global namespace is not 'near' its children.
+    Param.MaxUpTraversals = std::max(Path.second - 1, 0);
+    Sources[Path.first] = std::move(Param);
----------------
I think this sets MaxUpTraversals to -1 for the empty scope.
*probably* harmless in the end, but I think an explicit `if !S.empty()` might be clearer


================
Comment at: clangd/FileDistance.h:125
+private:
+  std::unique_ptr<FileDistance> Distance;
+};
----------------
nit: It seems slightly odd to make this a unique_ptr just to make the constructor simpler - could also just define a helper function in the CC file that the constructor could call: `ScopeDistance::ScopeDistance(ArrayRef<string> Scopes) : Distance(createScopeFileDistance(Scopes))`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53131