[libcxx-commits] [PATCH] D144734: [libcxx] Enable support for static and debug Windows runtimes

Mon Feb 27 05:14:41 PST 2023

mstorsjo added a comment.

> Testing on Windows requires compiler-rt builtins.

Currently it doesn't (the tests run without it), and regular use of libc++ in clang-cl mode wouldn't normally add that option. I know this would be useful for getting int128 handling fixed, but to get it fixed in practice and not only in tests, we'd also need to make sure that all users link with `-rtlib=compiler-rt` - and for users linking by invoking `link.exe` or `lld-link` directly instead of via the compiler wrapper, that's not happening. So this aspect is also a larger policy decision that needs to be split out and agreed upon by relevant stakeholders on its own, not just slipped into a giant refactoring patch.

Currently tests pass without this option, and I presume some tests that currently are expected to fail would start to work if we'd do this. Which tests are that? Or conversely, as tests demonstrably do work right now, what fails after your refactoring if you don't add it?

> For the shared DLL version of libc++ always use the DLL runtime because this is the only sensible choice.

I woulnd't entirely agree with that - all four combinations of {static,shared} libc++ vs {static,shared} CRT have their uses IMO.

In the end, it would be good to add CI coverage for at least some of the new CRT configurations. Or does this change imply that static libc++ now automatically uses static CRT instead of the dynamic one which is default so far? (This will change the outcome of some CI tests.)

Also finally, do note that out of 3 current clang-cl configurations in CI, this broke 2 entirely (one fails with `lld-link: error: could not open 'cxx_shared.lib': no such file or directory`, one fails with `fatal: unable to parse config file`) and the third one has a new failing test (`libcxx/vendor/clang-cl/static-lib-exports.sh.cpp`).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144734