[PATCH] D95188: [VFS] Fix inconsistencies between relative paths and fallthrough in the RedirectingFileSystem

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 18:07:04 PST 2021


dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: ormris.

This seems like a good simplification.



================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1085-1088
 std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
                                                bool &Result) {
   return ExternalFS->isLocal(Path, Result);
 }
----------------
I think you need to call `makeCanonical` here.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1748
 
-  // Canonicalize path by removing ".", "..", "./", components. This is
-  // a VFS request, do not bother about symlinks in the path components
-  // but canonicalize in order to perform the correct entry search.
-  Path = canonicalize(Path);
-  if (Path.empty())
+  // Some path operations expect a StirngRef.
+  StringRef PathStr = StringRef(Path.data(), Path.size());
----------------
Typo s/StirngRef/StringRef/


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1754
-  // but canonicalize in order to perform the correct entry search.
-  Path = canonicalize(Path);
-  if (Path.empty())
----------------
Is there a reason you had to manually inline `canonicalize()` here? It'd be nice to reuse the logic, since it's not entirely obvious.


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

https://reviews.llvm.org/D95188



More information about the llvm-commits mailing list