Non constexpr operator==

[mcskatkat]

First of all, great work.
This is probably the most thorough and streamlined backport of string_view publicly available, and I thank you for making it such.

I'm using it to replace Google's Abseil implementation, which still has some rough edges, and certainly drags in a lot of baggage for someone who just needs string_view.

One of my biggest motivations for using string_view is to facilitate string keyed associative containers with compile-time lookup, which opens the door to many useful things with zero runtime overhead. However, this requires constexpr comparison to be available...
This string_view delegates operator== to char_traits::compare() which is part of std, and as such is required NOT to be marked constexpr in pre-C++17 compilation. This is despite the compiler being perfectly able to perform constexpr comparisons.

In my case then, there is little value in backporting string_view if it doesn't backport its benefits too.

My workaround solution for this was deriving my own char_traits to implement a constexpr compare(), and then deriving my own string_view, as shown below. Perhaps you can think of a better way to backport constexpr comparison.

Thx

namespace my_own {
    namespace _detail {
        struct char_traits : public std::char_traits<char> {
            static constexpr int compare( const char_type* s1, const char_type* s2, std::size_t count ) {
                ... compiler specific ...
            }
        };
        using plain_string_view = nonstd::basic_string_view<char>;
    }
    class string_view : public nonstd::basic_string_view<char, _detail::char_traits> {
        using Base = nonstd::basic_string_view<char, _detail::char_traits>;
    public:
        using Base::Base;
        // need to provide own implicit conversions with std::string, since it uses the default char_traits
        operator string() const noexcept { return string(data(), size()); }
        string_view(string const & s) noexcept : Base(s.data(), s.size()) {}
        // provide implicit conversions with a string view that uses standard char_traits
        constexpr operator _detail::plain_string_view() const {return {data(), size()};}
        constexpr string_view(_detail::plain_string_view p) : Base(p.data(), p.size()) {}
    };
    inline std::ostream & operator<<(std::ostream & os, string_view sv) { return os << _detail::plain_string_view(sv); }
} // namespace

[martinmoene]

Hi @mcskatkat Thanks for your kind words. Also thanks for the interesting suggestion, i'll look into it.

[martinmoene]

Hi @mcskatkat can you have a look at above commit?

• would that work for you?
• should CTraits also provide constexpr eq()?
• ...

[mcskatkat]

Hi. Thanks for the quick response. This might work but there are two things about it:

1. You definitely want to use compiler builtin function to implement the comparison itself. It's very easy to get to thousands of comparisons made at compile time and strings may be long. Compilation might take an unnecessarily long time if implemented character by character. Note that compile time code execution is usually way slower than that of equivalent compiled code at run time.
   To that end, GCC provides __builtin_memcmp, and AFAIK MSVC and clang implement plain memcmp() calls as intrinsics, and all of them are constexpr. You can look at how Abseil handled this, or how the std lib sources handle it for C++17/20.
   BTW the same applies for strlen, which in your code is currently implemented character by character. I think it's best to leave the character by character implementation for cases where the compiler isn't identified / known.

2. I don't see the benefit of deriving CTraits from Traits if its only use is a single static function that's being called directly by fully qualified name. Such a function could just as well be a protected static member of string_view. The derivation was helpful for me trying to work around this from outside the class without modifying it, and it gave me an extra headache to reconcile the specialized string_view with the classes that use the default traits.

[mcskatkat]

Afterthought:

All of the above only applies when the element type is char, however that is by far the most common case.
So I would consider creating a protected static compare() in string_view with a specialization for char that delegates to compiler intrinsics if available. In all other cases fall back to the current element-by-element generic implementation.

[martinmoene]

Thanks for your notes, it is very helpful to get this steering based on actual use cases, drawing attention to weak spots and offering pointers to possible improvements.

I'll take it further with your suggestions.

[mcskatkat]

I edited my previous comment and removed incorrect items that resulted from me being lazy and only looking at the commit diff, missing the rest of the code.

[martinmoene]

I have the impression that builtins such as memcmp() and __builtin_memcmp(), __builtin_strlen() cannot be used in a constexpr context with g++ or msvc, like:

constexpr char hello[] = "hello";
constexpr char world[] = "world";

static_assert( string_view( hello ).compare( string_view( world ) ) == 0, "" );

See e.g. SO question Why does constexpr context make the compiler fail, while w/o it optimizes perfectly?.

On godbolt:

• clang (C++11), succeeds
• gcc (C++14), fails
• msvc (C++14), fails