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

use boost::core::string_view #2417

Closed
vinniefalco opened this issue Apr 27, 2022 · 13 comments
Closed

use boost::core::string_view #2417

vinniefalco opened this issue Apr 27, 2022 · 13 comments
Assignees

Comments

@vinniefalco
Copy link
Member

We should use boost::core::string_view instead of boost::utility::string_view, because it has better interoperability with std::string_view. However, before we make this change public we need to write a short matrix of tests that mix the three principle string views and the standard string std::string, std::string_view, boost::utility::string_view, boost::core::string_view in order to understand what compatibility problems users may experience (if any).

sehe added a commit to sehe/beast that referenced this issue May 2, 2022
This improves inter-conversion between string_view implementations. Some observable differences for users:
 - core::string_view no longer supports the .to_string() or .clear() extensions from Utility
 - code that relied on .max_size() returning .size(), needs to be fixed to .size() instead
 - remove_suffix() and remove_prefix() were more lenient than the standard specs; be sure you don't rely on it clamping the argument to valid range
 - BOOST_NO_CXX11_EXPLICIT_CONVERSION_OPERATORS no longer suppresses conversions to std::string
 - core::string_view adds .contains() and various bugs fixed

This also revealed several bugs against Boost Utility, and two feature
requests for Boost Core
@sehe
Copy link
Collaborator

sehe commented May 2, 2022

I've created a whole slew of test-cases to excavate differences between various implementations. These are not up to standard for inclusion, but perhaps someone sees some useful bits?

Goggles advised: https://github.com/sehe/beast/blob/core_string_view/test/beast/core/string.cpp

Note it uses c++17 for brevity - not in the last place to be able to compare with c++17 std::string_view, but liberally using contexpr and traits that are specific to c++17.

Differences between implementations that might be observed are marked OBSERVABLE

@sehe
Copy link
Collaborator

sehe commented May 4, 2022

@vinniefalco Just realized another loose end: despite std::string_view compatibility, core::string_view still doesn't support some templated free functions, like

  • std::quoted (too bad I guess)
  • asio::buffer
  • asio::const_buffer demo

The latter are a shame. They circle back to your discussion about custom buffer types in boost-http somewhat.

No regression or impact from the issue, just something we might want a solution to.

@sehe
Copy link
Collaborator

sehe commented May 5, 2022

Status:

  • IMO This should remain draft until boost::core::string_view is not hashable core#110 lands, to avoid inconveniencing Beast users with breaking change in their own code using beast::string_view

  • @vinniefalco / @pdimov Do you see any mechanism to aid with the compatiblity with std::string_view for free function templates (std::quoted, asio::buffer and possible others in thirdparty code)? Perhaps there's already a niebloid like beast::buffer_bytes that I'm not aware of for this?

@pdimov
Copy link
Member

pdimov commented May 5, 2022

What does "land" mean here?

@pdimov
Copy link
Member

pdimov commented May 5, 2022

What are we supposed to do about std::quoted or asio::const_buffer?

@sehe
Copy link
Collaborator

sehe commented May 5, 2022

The goal of this ticket is to make beast::string_view more compatible with std::string_view.

Both these free functions are std::string_view features, but not compatible with core::string_view. I'm just asking in case I'm missing some fancy way to address these?

@sehe
Copy link
Collaborator

sehe commented May 5, 2022

@pdimov "landing a ticket" means when it ships in the next release (it will merge to master, basically). Wasn't aware that was company specific lingo :/

@pdimov
Copy link
Member

pdimov commented May 5, 2022

It has been merged to master. Next release, well that's another story.

We can't change std::quoted, so I'm not sure what we can possibly do about it.

@vinniefalco
Copy link
Member Author

What the heck is std::quoted

@pdimov
Copy link
Member

pdimov commented May 5, 2022

It's a manipulator that quotes strings on op<< and unquotes them on op>>. https://godbolt.org/z/3srYsno9K

@sehe
Copy link
Collaborator

sehe commented May 5, 2022

@pdimov That's awesome. Yeah, I had expected the merge to be visible on boostorg/core#110 but that wasn't a PR in the first place. Are you saying that things that are on master do not /automatically/ get pulled into the superproject next release?

@pdimov
Copy link
Member

pdimov commented May 5, 2022

Yes, whatever's on master automatically goes into the next release. But the release itself is months from now.

@sehe
Copy link
Collaborator

sehe commented May 6, 2022

That's fine, the Beast change would be in the same release. As long as they go together or in the right order, I'm not in a hurry.

sehe added a commit to sehe/beast that referenced this issue May 8, 2022
This improves inter-conversion between string_view implementations. Some observable differences for users:
 - core::string_view no longer supports the .to_string() or .clear() extensions from Utility
 - code that relied on .max_size() returning .size(), needs to be fixed to .size() instead
 - remove_suffix() and remove_prefix() were more lenient than the standard specs; be sure you don't rely on it clamping the argument to valid range
 - BOOST_NO_CXX11_EXPLICIT_CONVERSION_OPERATORS no longer suppresses conversions to std::string
 - core::string_view adds .contains() and various bugs fixed
sehe added a commit to sehe/beast that referenced this issue May 8, 2022
fix boostorg#2417

This improves inter-conversion between string_view implementations. Some observable differences for users:
 - core::string_view no longer supports the .to_string() or .clear() extensions from Utility
 - code that relied on .max_size() returning .size(), needs to be fixed to .size() instead
 - remove_suffix() and remove_prefix() were more lenient than the standard specs; be sure you don't rely on it clamping the argument to valid range
 - BOOST_NO_CXX11_EXPLICIT_CONVERSION_OPERATORS no longer suppresses conversions to std::string
 - core::string_view adds .contains() and various bugs fixed
sehe added a commit to sehe/beast that referenced this issue May 9, 2022
fix boostorg#2417

This improves inter-conversion between string_view implementations. Some observable differences for users:
 - core::string_view no longer supports the .to_string() or .clear() extensions from Utility
 - code that relied on .max_size() returning .size(), needs to be fixed to .size() instead
 - remove_suffix() and remove_prefix() were more lenient than the standard specs; be sure you don't rely on it clamping the argument to valid range
 - BOOST_NO_CXX11_EXPLICIT_CONVERSION_OPERATORS no longer suppresses conversions to std::string
 - core::string_view adds .contains() and various bugs fixed
@sehe sehe closed this as completed in 9d23bec May 10, 2022
pinotree added a commit to pinotree/lager that referenced this issue Oct 6, 2023
Boost Beast 1.80 switches from boost::string_view to
boost::core::string_view [1][2], and thus the "to_string()" method to
convert to std::string is not available anymore.

Both the old and the new string_view types used support converting to
std::string via operator, hence switch to a static_cast to do the
conversion. This supports building with all the versions of boost.

[1] https://www.boost.org/doc/libs/release/libs/beast/doc/html/beast/release_notes.html
[2] boostorg/beast#2417
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

3 participants