[libcxx-commits] [PATCH] D98026: [libcxx] [test] Fix cross testing for windows
Martin Storsjö via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
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?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits