phosek added inline comments.
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1933-1935
path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
? sys::path::Style::posix
: sys::path::Style::windows_backslash;
----------------
haowei wrote:
> bnbarham wrote:
> > `windows_slash` is going to be `windows_backslash` here. I assume this was
> > fine since `sys::fs::make_absolute(Name)` would always return the native
> > style, but that isn't the case now. It's also unfortunate we calculate this
> > again when `makeAbsolute` only just did it.
> Thanks for pointing that out. I need to consider the windows forward slash
> case.
>
> There is a bit in consistency in the VFS implementation. for Windows. we can
> use forward slashes('/') on Windows. Most Windows API/Syscalls accept it. But
> some APIs may not work if you mix '/' with '\' in the path on Windows. What
> RedirectFileSystem::makeAbsolute is trying to do is, it first determine the
> slashes used in the WorkDir(either from process working directory or from
> command line flags, or OverlayDir), in which it can be a forward slash on
> Windows. Then it uses the same separator when appending the relative
> directory. Using sys::fs::make_absolute will break that.
>
> The reason that I use makeAbsolute here is that the OverlayDir will likely
> use forward slash on Windows if people uses CMake options
> LINK_FLAG="/vfsoverlay:C:/path/to/overlay.xml". In that case,
> sys::fs::make_absolute will generate some paths like C:/path/to/foo\\bar,
> which may cause issues.
>
> I will rewrite this to consider the forward slashes.
This was also discussed in D87732 which may provide some additional context.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137473/new/
https://reviews.llvm.org/D137473
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
