[libcxx-commits] [PATCH] D98026: [libcxx] [test] Fix cross testing for windows

Fri Mar 5 07:06:24 PST 2021

mstorsjo added a comment.

In D98026#2606651 <https://reviews.llvm.org/D98026#2606651>, @curdeius wrote:

> When cross-compiling (I imagine that you build on Linux and test on Windows), it seems logical to be using `WindowsRemoteTI`, no?
> I understand that it's not there and would be almost identical to `WindowsLocalTI`, but it would make things clearer IMO.

Yeah maybe, but so far I don't see the benefit in adding essentially a separate duplicate copy of each of these classes, when I so far haven't seen any case where it would warrant a difference between the local and remote versions. (We have one remote version so far, `LinuxRemoteTI`, and that one is functionally equivalent to `LinuxLocalTI`.) But I agree that it's an inconsistency that we have one such but not more than that.

> Another thing, wouldn't `LinuxRemoteTI` will have `is_windows` set to `True` when cross-compiling from Windows, and so need the analogical treatment as you do for Windows here?

Yes that's true, so maybe we should move all of the implementations of `is_windows`, `is_darwin` etc into the platform specific subclasses, and have the base class versions only return false.

Maybe it'd be best if I split this patch into two; one for just fixing clang-cl matching, and one for moving the `is_*` functions into subclasses?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98026