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

[Jordan Rupprecht via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338157: [libc++] Exclude posix_l/strtonum fallback inclusion 
for newlib  2.4 (authored by rupprecht, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D49927

Files:
  libcxx/trunk/include/support/newlib/xlocale.h


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -19,9 +19,9 @@
 #if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
 __NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
-#endif
 #include 
 #include 
+#endif
 
 #endif // _NEWLIB_VERSION
 


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -19,9 +19,9 @@
 #if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
 __NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
-#endif
 #include 
 #include 
+#endif
 
 #endif // _NEWLIB_VERSION
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In https://reviews.llvm.org/D49927#1178659, @ldionne wrote:

> 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.




Repository:
  rCXX libc++

https://reviews.llvm.org/D49927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rCXX libc++

https://reviews.llvm.org/D49927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

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.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht created this revision.
rupprecht added reviewers: ldionne, rsmith.
Herald added a reviewer: EricWF.
Herald added subscribers: cfe-commits, christof.

[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.4 [1] [2], 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


Repository:
  rCXX libc++

https://reviews.llvm.org/D49927

Files:
  include/support/newlib/xlocale.h


Index: include/support/newlib/xlocale.h
===
--- include/support/newlib/xlocale.h
+++ include/support/newlib/xlocale.h
@@ -19,9 +19,9 @@
 #if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
 __NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
-#endif
 #include 
 #include 
+#endif
 
 #endif // _NEWLIB_VERSION
 


Index: include/support/newlib/xlocale.h
===
--- include/support/newlib/xlocale.h
+++ include/support/newlib/xlocale.h
@@ -19,9 +19,9 @@
 #if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
 __NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
-#endif
 #include 
 #include 
+#endif
 
 #endif // _NEWLIB_VERSION
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits