rnk accepted this revision.
rnk added a subscriber: JDevlieghere.
rnk added a comment.
This revision is now accepted and ready to land.
+@JDevlieghere, due to recent VFS test fixup.
I think this looks good, and in the absence of input from other VFS
stakeholders, let's land this soon. Thanks!
================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char>
&Path) const {
+ if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+ llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+ return {};
----------------
rnk wrote:
> amccarth wrote:
> > amccarth wrote:
> > > rnk wrote:
> > > > I think Posix-style paths are considered absolute, even on Windows. The
> > > > opposite isn't true, a path with a drive letter is considered to be
> > > > relative if the default path style is Posix. But, I don't think that
> > > > matters. We only end up in this mixed path style situation on Windows.
> > > >
> > > > Does this change end up being necessary? I would expect the existing
> > > > implementation of makeAbsolute to do the right thing on Windows as is.
> > > > I think the other change seems to be what matters.
> > > Yes, it's necessary. The Posix-style path `\tests` is not considered
> > > absolute on Windows. Thus the original makeAboslute would merge it with
> > > the current working directory, which gives it a drive letter, like
> > > `D:\tests\`. The drive letter component then causes comparisons to fail.
> > To make sure I wasn't misremembering or hallucinating, I double-checked the
> > behavior here: Posix-style paths like `\tests` are not considered absolute
> > paths in Windows-style.
> I see, I agree, the platforms diverge here:
> bool rootDir = has_root_directory(p, style);
> bool rootName =
> (real_style(style) != Style::windows) || has_root_name(p, style);
>
> return rootDir && rootName;
>
> So, on Windows rootDir is true and rootName is false.
>
> I still wonder if this behavior shouldn't be pushed down into the base class.
> If I pass the path `\test` to the real `FileSystem::makeAbsolute` on Windows,
> should that prepend the CWD, or should it leave the path alone?
I think we discussed this verbally, and decided we should prepend the CWD, as
is done here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70701/new/
https://reviews.llvm.org/D70701
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
