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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits