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

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D48687#1146515, @simark wrote:

> In https://reviews.llvm.org/D48687#1146308, @ilya-biryukov wrote:
>
> > 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?
>
>
> I tried to do that, but it goes deep in the clang internals with which I'm not familiar.  All I could see was that when creating the `FileEntry` representing the `/home/emaisin/src/ls-interact/cpp-test/build/../src/first.h` file, `FileManager::getFile` is called with `OpenFile=false`.  This makes it so that the `RealPathName` field is not set (at `FileManager.cpp:320`).  Because `RealPathName` is not set (well, empty), we use the non-normalized name in `getAbsoluteFilePath`. That's all I can tell.


I think from what we've seen before, it had to do with location coming from the preamble vs not. Simon, maybe we can spend some time together to investigate this. I know I said I'd rather not touch the Clang part as I thought it might be on purpose and/or performance sensitive, but the fact that the locations are inconsistent between the preamble and outside...it's not ideal and error-prone. Either way I think we need this fix in Clangd as a defensive fix because it's not in the AST's contract/guarantees (preamble or not) to provide locations with realpaths.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687