Re: [PATCH v3 3/5] Add gdb::string_view

[Pedro Alves <palves@redhat.com>]

On 04/07/2018 02:26 PM, Simon Marchi wrote:
> New in v3:
> 
>   * Base on the implementation in experimental/, since it is easier to
>   adapt to the c++ standard we follow (c++11).  I didn't see any
>   significant fix to import from the std/ version.
> 
>   * Tested against clang and clang+libc++ to make sure we don't rely on
>   anything internal and specific to libstdc++.  This caught a few
>   issues.
> 
>   * Remove hash helpers, operator<<, operator ""sv.

We can always add hash support later, but I suspect it won't be
long until we will miss it, when we try to use string_view with
e.g., an std::unordered_map.

>   * Make gdb::string_view an alias of std::string_view when building
>   with >= c++17.
> 
>   * Remove a bunch of constexpr, because they are not valid in c++11
>   (e.g. they are not a single return line).

I think I'd prefer to leave the "constexpr"'s in place, but commented out,
instead of just removing it, like '/*constexpr*/', as a clearer marker that
we've removed something that normally would otherwise be there, since for
a future reader, simply removing is indistinguishable with missing/forgetting
a constexpr.

>   * Remove hash helpers, because they use libstdc++ internal functions.
>   If we need them we always import them later.

Ah, hash helpers mentioned twice. :-)

Technically, the use of uglified (double-underscore or
underscore-followed-by-uppercase) names is undefined behavior for
being reserved to the implementation, but I think that in practice
it's harmless, we should be able to deal with any conflict if it
ever happens.  At this stage, I think fewer local changes outweighs
the very small potential for trouble.

LGTM.

Thanks,
Pedro Alves