This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Make constexpr char_traits<T> and char_traits<char>
Needs RevisionPublic

Authored by AntonBikineev on Nov 20 2016, 8:00 AM.

Details

Summary

This implements constexpr assign, find, compare and length functions proposed in p0426r1.
Synopsis:

static constexpr void assign(char_type& c1, const char_type& c2) noexcept;
static constexpr int compare(const char_type* s1, const char_type* s2, size_t n);
static constexpr size_t length(const char_type* s);
static constexpr const char_type* find(const char_type* s, size_t n, const char_type& a);

Note that only primary template char_traits<T> and char_traits<char> specialization are addressed for now...

Diff Detail

Event Timeline

AntonBikineev retitled this revision from to [libcxx] Make constexpr char_traits<T> and char_traits<char>.
AntonBikineev updated this object.
AntonBikineev added reviewers: mclow.lists, EricWF.
AntonBikineev added a reviewer: cfe-commits.
mclow.lists edited edge metadata.Nov 21 2016, 1:03 PM

Tests look good; I'm still staring at the stuff in <__string>.

include/__string
138

Let's pick an order and stick to it.

Either _LIBCPP_CONSTEXPR_AFTER_CXX14 inline (as we use here)
or inline _LIBCPP_CONSTEXPR_AFTER_CXX14 (as we use on L#204)

I prefer inline _LIBCPP_CONSTEXPR_AFTER_CXX14.

AntonBikineev edited edge metadata.
EricWF edited edge metadata.Nov 23 2016, 1:45 AM

Could you please generate this patch with more context? (ie with -U999 or similar).

include/__config
925

What about GCC? Surely it implements some if not most of these.

EricWF added inline comments.Nov 23 2016, 1:47 AM
include/__string
213–282

wow. This is #ifdef hell. Please find a way to do it with less (or hopefully no) conditional compilation blocks.

AntonBikineev edited edge metadata.

Support gcc's __builtin_memcpy, memchr, strlen

AntonBikineev marked 2 inline comments as done.Nov 24 2016, 5:10 AM
AntonBikineev added inline comments.
include/__config
925

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.

include/__string
213–282

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

mclow.lists added inline comments.Nov 28 2016, 10:56 AM
include/__string
213–282

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.

EricWF added a comment.Dec 5 2016, 1:34 AM

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

include/__config
925

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.

include/__string
213–282

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.

EricWF requested changes to this revision.Dec 10 2016, 11:58 PM
EricWF edited edge metadata.

Marking as "Changes requested". I'll take a look at this again once the if-def hell around find and compare are cleaned up.

This revision now requires changes to proceed.Dec 10 2016, 11:58 PM