[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 marked 3 inline comments as done.
ArcsinX added a comment.

As far as I do not have commit access, could you please commit for me?
Aleksandr Platonov <platonov.aleksandr at huawei.com>



================
Comment at: clang/lib/Tooling/FileMatchTrie.cpp:111
+      // As far as we do not support file symlinks we compare
+      // basenames here to avoid expensive request to file system.
+      if (llvm::sys::path::filename(Path) ==
----------------
sammccall wrote:
> Here it's really just for consistency - we have a single candidate, and calling equivalent() on a single file isn't expensive. I'd be happy with or without this check, but the comment should mention consistency.
Removed "expensive" word.


================
Comment at: clang/lib/Tooling/FileMatchTrie.cpp:131
+    // We failed to find the file with `Children.find()`.
+    // If `ConsumedLength` is equal to 0 then we have tried to find the file
+    // with it's basename. Thus, we do not have files with the same
----------------
sammccall wrote:
> nit: This is quite a lot of words to come to "we do not support file symlinks" - I think it could be a bit shorter and also get into motivation:
> 
> "If `ConsumedLength` is zero, this is the root and we have no filename match. Give up in this case, we don't try to find symlinks with different names".
Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83621