[PATCH] D111457: [clang][test] Add lit helper for windows paths

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

In D111457#3082398 <https://reviews.llvm.org/D111457#3082398>, @mstorsjo wrote:

> In D111457#3073726 <https://reviews.llvm.org/D111457#3073726>, @keith wrote:
>
>> In D111457#3066508 <https://reviews.llvm.org/D111457#3066508>, @mstorsjo wrote:
>>
>>> Wouldn't this one also be solved pretty much the same, but differently, by changing `if (llvm::sys::path::is_absolute(RemappedFile)) {` into `is_absolute_gnu`?
>>
>> I think it //could// but also users could still remap to native window's paths, so likely we'd want this test as well I think either way?
>
> I guess this might be a good addition as a new test, yeah, but I think it would be good to keep this test as is too, and change the code to use `is_absolute_gnu` (and fix up the test reference here to expect an empty directory in the output).
>
>>> Since we're remapping debug paths, it's plausible that the target path can be a different style (when cross compiling, where debug prefix remapping is kinda important), and then it's probably good to use a more lax definition of whether a path is absolute. (Alternatively, we maybe should try to detect the kind of path used and use the appropriate style as argument to `is_absolute`, but I don't think that necessarily helps here.)
>>
>> Good point here, I could definitely see wanting to support the entire matrix of host windows paths vs not, and target windows paths vs not, but I think that would require significantly more changes for other places that call `llvm::sys::path::*` APIs and also use the default `native` argument (I'm not sure how difficult this would be, but it would require a bit of auditing)
>
> I would expect that to mostly work so far, except for corner cases like these. Auditing probably doesn't hurt if one wants to spend the effort, otherwise I'd just expect it to work and try it out and see if one runs into any issues somewhere, if someone has such a usecase.

Yep this makes sense. I would rather not scope that into this if that's ok. Since this test was already invalid and broken (actual fix in https://reviews.llvm.org/D111579) I would rather land these and then take that on to unblock my original use case (https://reviews.llvm.org/D111587) if I can find some more time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111457