[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D83621#2151716 <https://reviews.llvm.org/D83621#2151716>, @sammccall wrote:

> In D83621#2146750 <https://reviews.llvm.org/D83621#2146750>, @ArcsinX wrote:
>
> > > - don't scan for equivalences if the set of candidates exceeds some size threshold (10 or so)
> > > - don't scan for equivalences if the node we'd scan under is the root
> >
> > After such fixes `JSONCompilationDatabase::getCompileCommands()` will return other results than before in some cases.
>
>
> Can you elaborate on this - do you think there are reasonable cases where it will give the wrong answer?
>  I can certainly imagine cases where this changes behaviour, e.g. project/foo.cc is a symlink to project/bar.cc, compile_commands contains an entry for foo.cc, we query for bar.cc.
>  But I don't think this was ever really intended to handle arbitrary symlinks that change the names of files, and I do think this is OK to break.


If breaking `project/foo.cc <=> project/bar.cc` is OK, then could we also break `some/path/here/foo.cc <=> some/other/path/bar.cc`?

In that case we can just prevent `Comparator.equivalent(FileA,FileB)` calls when `llvm::sys::path::filename(FileA) != llvm::sys::path::filename(FileB)`.
I tried this locally:

- it works fast.
- does not break tests.
- easy to add unit test for `FileMatchTrie`.

What do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83621/new/

https://reviews.llvm.org/D83621