This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4
ClosedPublic

Authored by rupprecht on Jul 27 2018, 12:06 PM.

Details

Summary

[libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

r338122 changed the linkage of some methods which revealed an existing ODR violation, e.g.:

projects/libcxx/include/support/xlocale/__posix_l_fallback.h:83:38: error: 'internal_linkage' attribute does not appear on the first declaration of 'iswcntrl_l'
inline _LIBCPP_INLINE_VISIBILITY int iswcntrl_l(wint_t c, locale_t) {
                                     ^
lib/include/wctype.h:55:12: note: previous definition is here
extern int      iswcntrl_l (wint_t, locale_t);

These were added to newlib in 2.5 [1] [2] (newlib version is 2.4 in the tree when committed), so move them to the already existing include guard.

[1] https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=commit;h=238455adfab4f8070ac65400aac22bb8a9e502fc
[2] https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=commit;h=8493c1631643fada62384768408852bc0fa6ff44

Diff Detail

Repository
rL LLVM

Event Timeline

rupprecht created this revision.Jul 27 2018, 12:06 PM
rupprecht edited the summary of this revision. (Show Details)Jul 27 2018, 12:07 PM

Just to make sure I understand properly: this means we will use newlib's implementation of iswcntrl_l & friends instead of our own emulation (which is an ODR violation currently going unnoticed)? And this is OK because newlib provides iswcntrl_l & friends starting at version 2.4.

This is my understanding. If it is correct, then LGTM.

rupprecht edited the summary of this revision. (Show Details)Jul 27 2018, 12:17 PM
jyknight accepted this revision.Jul 27 2018, 12:22 PM
jyknight added a subscriber: jyknight.

Typo in commit message? They were added to 2.5, not 2.4 (the code is right, just the comment is wrong).

This revision is now accepted and ready to land.Jul 27 2018, 12:22 PM

Just to make sure I understand properly: this means we will use newlib's implementation of iswcntrl_l & friends instead of our own emulation (which is an ODR violation currently going unnoticed)? And this is OK because newlib provides iswcntrl_l & friends starting at version 2.4.

Yes, that's correct.
I clarified the description: when the methods were added, the tree was configured with NEWLIB_MINOR_VERSION = 4, meaning this wasn't released until the next version, newlib 2.5.

This is my understanding. If it is correct, then LGTM.

This revision was automatically updated to reflect the committed changes.