EricWF added a comment.

The tests LGTM. The implementation still needs some tweaks. Thanks for working 
on this.



================
Comment at: include/__config:925
 
+#if !__has_builtin(__builtin_memcpy)
+#define _LIBCPP_HAS_NO_BUILTIN_MEMCPY
----------------
AntonBikineev wrote:
> EricWF wrote:
> > What about GCC? Surely it implements some if not most of these.
> Thanks, good point. I've done it in the same way as it is done for 
> __buitin_addressof. On the other hand, gcc has an option -fno-builtin to 
> disable them. That way it *won't* compile. I try to stick to the current 
> solution though.
A couple of problems with your usage of `_GNUC_VER`.

First `_GNUC_VER` is always 421 when using Clang, so the above check never 
fails with Clang.
Second we require at least GCC 4.8/4.9. Don't bother configuring for anything 
older than that.


================
Comment at: include/__string:213
 
-    static inline int compare(const char_type* __s1, const char_type* __s2, 
size_t __n) _NOEXCEPT
-        {return __n == 0 ? 0 : memcmp(__s1, __s2, __n);}
-    static inline size_t length(const char_type* __s)  _NOEXCEPT {return 
strlen(__s);}
-    static inline const char_type* find(const char_type* __s, size_t __n, 
const char_type& __a) _NOEXCEPT
-        {return __n == 0 ? NULL : (const char_type*) memchr(__s, 
to_int_type(__a), __n);}
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_CXX14_CONSTEXPR)
+    static inline constexpr int
----------------
mclow.lists wrote:
> AntonBikineev wrote:
> > EricWF wrote:
> > > wow. This is #ifdef hell. Please find a way to do it with less (or 
> > > hopefully no) conditional compilation blocks.
> > yep, this is generic hell. I want to cover as many cases as possible, i.e. 
> > combinations of (is_constexpr x has_builtin_xxx) for every function. I'm 
> > open to suggestions
> How about (for compare, say) you just forget about `memcmp`.  Either call 
> `__builtin_memcmp` if it is available, or use a hand-rolled loop.
> 
> Note: gcc has had `__builtin_memcmp` since at least 4.8. (and it is constexpr)
> 
> And just mark the function with `_LIBCPP_CONSTEXPR_AFTER_CXX14`.
Exactly what @mclow.lists said. That seems like the correct solution. We're 
*always* going to have `__builtin_memcmp` for all of our supported compilers 
AFAIK.


https://reviews.llvm.org/D26896



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

Reply via email to