[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Only a few NITs from my side.
Excited for this fix to get in, have been seeing duplicate in other cases too :-)



================
Comment at: clangd/SourceCode.h:69
 
+llvm::Optional<std::string> getRealPath(const FileEntry *F,
+                                        const SourceManager &SourceMgr);
----------------
This function looks like a good default choice for normalizing paths before putting them into LSP structs, ClangdServer responses, etc.
I suggest we add a small comment here with a guideline for everyone to attempt using it whenever possible. WDYT?


================
Comment at: unittests/clangd/TestFS.h:43
 public:
-  /// When \p UseRelPaths is true, uses relative paths in compile commands.
-  /// When \p UseRelPaths is false, uses absoulte paths.
-  MockCompilationDatabase(bool UseRelPaths = false);
+  /// If \p Directory is not null, use that as the Directory field of the
+  /// CompileCommand.
----------------
s/not null/not empty


================
Comment at: unittests/clangd/TestFS.h:46
+  ///
+  /// If \p RelPathPrefix is not null, use that as a prefix in front of the
+  /// source file name, instead of using an absolute path.
----------------
s/not null/not empty


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687