<div dir="ltr">BTW, I hope I didn't come across as overly negative in my previous response. I'd love to see the situation improved. I just don't know what that would look like.<div><br></div><div>> One thing I see, but wasn't obvious from your description, is that the mixed/hybrid separator behaviour only happens for `defined(_WIN_32)`.</div><div>> [copy of `is_separator` elided]</div><div><br></div><div>It's a runtime decision based on the specified style, not a compile-time one based on _WIN_32. If the caller of `is_separator` passes `Style::windows` then it'll accept either `/` or `\\`, even if LLVM was compiled and run on Linux or Mac. I believe there's a VFS test that redirects a Windows-style path to a Posix-style one (an in-memory file system), and that test passes on both kinds of hosts.</div><div><br></div><div>But I get the gist of the point. My feeling is that, unless we can eliminate hybrid styles, Paths should support a Style::hybrid. It would be messy because more ambiguities about how to map things crop up.</div><div><br></div><div><span style="color:rgb(80,0,80)">> Honestly, I'm not sure we have a good definition of what makeAbsolute</span><br style="color:rgb(80,0,80)"><span style="color:rgb(80,0,80)">> should do.</span><br></div><div><span style="color:rgb(80,0,80)"><br></span></div>I should have said: **I** don't have a good understanding of what makeAbsolute should do. Even saying it should prepend the current working directory is an incomplete answer. On Windows, a process can have multiple current directories: one for each drive. And a process has one current drive. So the current working directory is the current directory for the current drive. A Windows path like "D:foo.txt" is a relative path. Literally prepending the current working directory gives us `C:\users\me\D:foo.txt`, which is syntactically wrong. But even if we're smart enough to fix up the syntax, we'd get `C:\users\me\foo.txt` or `D:\users\me\foo.txt`, both of which would (likely) be wrong. The way the OS would resolve it is to look up the process's current directory for the D: drive, and insert that into the missing bit.<div><br></div><div>Anyway, I look forward to any and all proposals to improve this situation.<br><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 21, 2021 at 6:47 PM Duncan P. N. Exon Smith via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">dexonsmith added a comment.<br>
<br>
Thanks for the quick and detailed response!<br>
<br>
Your explanation of hybrid behaviour on Windows was especially helpful, as I didn't fully understand how that worked before.<br>
<br>
One thing I see, but wasn't obvious from your description, is that the mixed/hybrid separator behaviour only happens for `defined(_WIN_32)`. E.g., from Path.cpp:<br>
<br>
bool is_separator(char value, Style style) {<br>
if (value == '/')<br>
return true;<br>
if (real_style(style) == Style::windows)<br>
return value == '\\';<br>
return false;<br>
}<br>
<br>
<br>
<br>
In D70701#2514143 <<a href="https://reviews.llvm.org/D70701#2514143" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70701#2514143</a>>, @amccarth wrote:<br>
<br>
>> - The path style is detected when reading the YAML file (as now).<br>
><br>
> Which path's style? The virtual one that's going to be redirected or the<br>
> actual one it's redirected at?<br>
<br>
Both, but you've mostly convinced me not to go down this route.<br>
<br>
> - Paths in the YAML file are canonicalized to native at parse time.<br>
><br>
> If we canonicalize the virtual path, VFS would no longer be valuable for<br>
> creating platform-agnostic tests.<br>
<br>
This is a good point I hadn't considered.<br>
<br>
> In LLDB, we have the additional wrinkle of remote debugging, where the<br>
> debugger may be running on a Windows machine while the program being<br>
> debugged is running on a Linux box. You always have to know whether a path<br>
> will be used on the debugger host or the debuggee host. And there are<br>
> similar concerns for post-mortem debugging from a crash collected on a<br>
> different type of host.<br>
<br>
Ah, interesting.<br>
<br>
> Honestly, I'm not sure we have a good definition of what makeAbsolute<br>
> should do.<br>
<br>
Perhaps the name isn't ideal -- `prependRelativePathsWithCurrentWorkingDirectory()` would be more precise -- but otherwise I'm not sure I fully agree. Regardless, I acknowledge your point that the two worlds are awkwardly different.<br>
<br>
I'm going to think about other options; thanks again for your feedback. I am still leaning toward `FileSystem::makeAbsolute()` not being virtual / overridden, but I have a better idea of how to approach that. One idea is for the `RedirectingFileSystem` to keep track of where different styles were used when parsing.... I'm not sure if that'll pan out though.<br>
<br>
<br>
Repository:<br>
rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D70701/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70701/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D70701" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70701</a><br>
<br>
</blockquote></div>