[PATCH] D74488: [VFS] Fix vfsoverlay assertion due to RedirectingFileSystem path handling.

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

I'm stumped why the regular tests don't repro this for me.  Somehow, getDirectoryFromFile is only ever called with a fully resolved path, so it never falls back to setting DirName to `.`.  I'm going to stop chasing this.  I can make it happen with your zipped example.

I would prefer to solve the problem by making canonicalize smarter rather than bypassing it.  I have a few ideas:

1. Instead of just checking the first slash type, it would first check for a drive letter, which obviously means it's Windows style.  If there's no drive letter, fall back to slash check.  In the problematic case, it's always an absolute path, so this should work.

2. Have canonicalize resolve dots and double dots itself rather than delegating it to the sys::path::remove_dots and sys::path::remove_leading_dots.  This could be done in a slash agnostic (and preserving) way rather than trying to guess the style.  The problem boils down to the fact that we have paths in a hybrid style that sys::path was never intended to handle.

3. Fix the test so that it doesn't convert the backslashes to slashes and make the cdb and yaml parsing reject hybrid-style paths.  That's probably the most principled thing, but it's also the most fragile and it's not consistent with Clang's general hands-off policy when it comes to manipulating paths.

I'm leaning toward 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74488