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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the patch! 
Could we try to figure out why the duplicates were there in the first place and why the paths were different?
It should be easy to mock exactly the same setup you have in #37963, i.e. create a vfs with three files and compilation database that has exactly those compile commands. You mention you tried to repro the issue, did you try doing that with the unit-test or a lit-test?

After looking at the makefile, my guess would be that the problem comes from the paths starting with `../` inside the compilation database.



================
Comment at: clangd/XRefs.cpp:179
+                                                const SourceManager &SourceMgr,
+                                                const vfs::FileSystem &VFS) {
   SmallString<64> FilePath = F->tryGetRealPathName();
----------------
Do we really need an extra vfs param?
Could we use `SourceMgr.getFileManager().getVirtualFileSystem()` instead?




================
Comment at: unittests/clangd/TestTU.h:47
 
-  ParsedAST build() const;
+  ParsedAST build(IntrusiveRefCntPtr<vfs::FileSystem> *OutFS = nullptr) const;
   SymbolSlab headerSymbols() const;
----------------
We don't need an extra output param here. 
There's a way to get the vfs from the ASTContext: `ParsedAST().getASTContext().getSourceManager().getFileManager().getVirtualFileSystem()`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687