The code has been quite ready for a while now and there are no more ABI
breaking papers. So this is a good time to mark the feature as stable.
Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rGdff62f5251f2: [libc++][format] Removes the experimental status.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This LGTM w/ comments.
Thanks a lot for all the work you've put into taking std::format from nothing to a stable feature, this is awesome!
libcxx/test/libcxx/experimental/fexperimental-library.compile.pass.cpp | ||
---|---|---|
26–30 | This should be removed in this patch. | |
30 | @philnik We should add a test for the PSTL here, that's an oversight. This would make sure that the PSTL is indeed enabled when we pass -fexperimental-library. Then it makes sense to remove _LIBCPP_HAS_NO_INCOMPLETE_FORMAT and this test still has a purpose. I think that we are likely to have at least one experimental feature in flight at all times, so this test would stay relevant. |
Sharing in case anyone else runs into the same problem: This caused Chromium builds to fail on Windows due to not linking against compiler-rt's builtins library. It turns out this patch caused us to compile some code doing 128-bit arithmetic and calling the __udivti3 runtime function. We worked around it by defining _LIBCPP_HAS_NO_INT128 until we can make the builtins library part of all our builds.
Interesting that this caused it. I expect the real cause is std::to_char for 128-bit values, there divisions are used. Does that mean the flag in __config is not set correctly? It was recently updated in D134912.
It's tricky. As discussed on that change, Clang does support 128-bit operations also on Windows so the flag is correct in that sense, but it requires linking with the builtins library which we don't do (yet).
I see. Is there an estimate how long it takes to get the linking with the builtins library will take. I wonder whether we should ship libc++17 with 128 bit support on Windows when it may lead to link time error.
I think there's a good chance of it happening this year, but probably not before the 17 branch which I'm guessing should happen pretty soon.
@ldionne you added this file, do we want to keep it? If so we probably need to generated something. I don't think there is a similar PSTL test.