[libcxx-commits] [PATCH] D138195: [libc++] Fix __regex_word value when using newlib/picolibc

Fri Nov 18 10:20:52 PST 2022

philnik added subscribers: jfb, jroelofs.
philnik added a comment.

In D138195#3937494 <https://reviews.llvm.org/D138195#3937494>, @arichardson wrote:

> In D138195#3937471 <https://reviews.llvm.org/D138195#3937471>, @philnik wrote:
>
>> In D138195#3937400 <https://reviews.llvm.org/D138195#3937400>, @michaelplatings wrote:
>>
>>> I think this could be considered an ABI break. I don't see an alternative and given that the static assert has been failing for some time maybe no-one cares. But just in case, would it be worth noting in the release notes <https://github.com/llvm/llvm-project/blob/main/libcxx/docs/ReleaseNotes.rst#abi-affecting-changes>?
>>
>> This is definitely an ABI break, but I'm not sure we care for newlib about that. We don't officially support it and most people that are using newlib + libc++ are probably in an environment where ABI stability isn't much of a concern. It's mostly used in embedded environments, right? It's probably a good idea to add a note to the `ABI Affecting Changes` section though.
>
> I agree this is technically an ABI break, but I believe there can't be any affected code since the static_assert would have failed previously when including <regex> for a libc++ build with locale support? And even if it is, I would assume this should not matter for newlib where you generally don't have a system libc++.so that is being linked against.
> Happy to add a release note though.

The `static_assert` was only added in LLVM 14, so there could be users that simply haven't upgraded yet. Anyways, I think this patch LGTM with a release note. @jfb @jroelofs it looks like you worked a bit on newlib + libc++ support. Do you have any opinion here? #libc_vendors <https://reviews.llvm.org/tag/libc_vendors/> is anybody using newlib with libc++? If yes, what's your opinion?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138195