[libcxx-commits] [PATCH] D134912: [libc++] Disable int128_t and ship filesystem on Windows by default

Sat Oct 1 02:43:19 PDT 2022

mstorsjo added a subscriber: MaskRay.
mstorsjo added a comment.

In D134912#3828421 <https://reviews.llvm.org/D134912#3828421>, @rnk wrote:

> In D134912#3826856 <https://reviews.llvm.org/D134912#3826856>, @ldionne wrote:
>
>> In D134912#3825356 <https://reviews.llvm.org/D134912#3825356>, @mstorsjo wrote:
>>
>>> Generally, `s/Windows/MSVC/` for the whole commit message,
>>
>> Right, I'll fix that.
>>
>> I know @rnk had discussed shipping `int128_t` support a while back, I'd like to hear from him whether there are plans to do this in the short term (the next few weeks). Otherwise, we'll ship this and take a break if we want to use `int128_t` support in filesystem in the future (which I guess we don't have to). We have other similar situations w.r.t. ranges already FWIW (with `no_unique_address`), so there is precedent for not being fully ABI stable on Windows.
>
> Sorry, I think this task was assigned to someone who has since left our team, and it's not likely to happen soon.
>
> I think the way forward here is to add `/DEFAULTLIB:clang_rt.builtins-${arch}.lib` to LLVM objects when i128 is present. I think we have some prior art for this around the _fltused symbol.

I think that might work. MSVC does ship those libraries (in `VC/Tools/MSVC/<version>/lib/<arch>` today - however, they only ship them for `x86` and `x64` - they're missing for `arm` and `arm64`, so we'd need to file a request for them to start shipping those files (at least builtins, the sanitizers aren't equally essential) for all architectures.

If considering libraries shipped with Clang as opposed to ones shipped with MSVC, we already have prior art saying that the user may need to specify the path to the Clang runtime library directory manually to the linker, since b8000c0ce84541c5b5535419234fb65ce77d6756 <https://reviews.llvm.org/rGb8000c0ce84541c5b5535419234fb65ce77d6756>. Previously that only extended to when the user knowingly was using sanitizers, now it would also happen when unwittingly using `int128_t`.

There's a potential issue ahead, if we're switching the library naming due to `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` (CC @phosek @MaskRay), since we probably don't know which library naming is the preferred one at the point where we're injecting the defaultlib directive.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134912