Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constexpr basic_string_view #24

Open
1 task done
andrewerf opened this issue Oct 11, 2021 · 1 comment
Open
1 task done

Constexpr basic_string_view #24

andrewerf opened this issue Oct 11, 2021 · 1 comment

Comments

@andrewerf
Copy link

andrewerf commented Oct 11, 2021

Checklist

  • I did not find a duplicate of this feature request in the Github Issues section.

Description

The bpstd::basic_string_view uses std::char_traits as a background. This produces a error in a constexpr context:
error: call to non-'constexpr' function 'static std::size_t std::char_traits<char>::length(const char_type*)'.
This is correspond to the standard because char_traits::length is marked constexpr since C++17 only.

P.S. Idk whether it's a bug or a feature request.

@bitwizeshift
Copy link
Owner

Hi @andrewerf, sorry for not getting back to this sooner. This is a good issue to raise, but unfortunately not one with a clear-cut solution, which is why it is the way it is currently.

As you have noticed, std::char_traits::length is only constexpr in C++17 and above. For string_view to be constexpr, you need a CharTraits argument for basic_string_view that is also constexpr. One option would be for Backport to implement a custom bpstd::char_traits that is constexpr to support this -- but then this introduces a whole new set of issues. The problem arises that std::string::char_traits will not be the same as bpstd::string_view::char_traits, since we would be defining a custom char_traits. Not only would this affect any template magic that detects T::char_traits for correctness, but this will also negatively impact std::string's explicit conversion from string_view.

In C++17, std::basic_string learned support for explicit conversion from any T type where std::is_convertible_v<const T&, std::basic_string_view<CharT, Traits>> is true. To replicate this in Backport as best as possible, I've given basic_string_view an explicit conversion operator to std::basic_string<...,Allocator>. This doesn't perform any T to std::string_view conversions -- but it's a limited/reasonable subset to produce somewhat similar behavior, and it also saves having to implement/wrap all of string in Backport.

If a new bpstd::char_traits type were added -- then this conversion would now be ill-formed because std::char_traits is not the same as bpstd::char_traits. The only way to fix this would either be for Backport to relax conversion requirements -- which can be bad/buggy/unportable to std::string_view, or Backport would need to implement basic_string and have it also be in terms of char_traits -- which is also really not so nice.

Ultimately I figured the best way to work around this is to simply require users to define a constexpr version of char_traits if this were needed at compile-time. This way, in general, string_view works as expected with normal std::string objects -- but if constexpr is needed in pre-c++17, then it's an opt-in choice.


If you have any other suggested ways of dealing with this outside of what I mentioned above, I would be interested to discuss and possibly implement it -- but otherwise my best recommendation would just be to own a custom constexpr_char_traits that you use when constexpr support is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants