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

Issue #2417 use boost::core::string_view #2418

Merged
merged 1 commit into from
May 10, 2022
Merged

Conversation

sehe
Copy link
Collaborator

@sehe sehe commented 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

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://2418.beastdocs.prtest.cppalliance.org/libs/beast/doc/html/index.html

@cppalliance-bot
Copy link

pullrequest
Timeline tracing charts: https://2418.beast.tracing.cppalliance.org/index.html

@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #2418 (2d7b5b5) into develop (90e37ae) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2418   +/-   ##
========================================
  Coverage    94.66%   94.67%           
========================================
  Files          149      149           
  Lines        11773    11773           
========================================
+ Hits         11145    11146    +1     
+ Misses         628      627    -1     
Impacted Files Coverage Δ
include/boost/beast/core/string_type.hpp 100.00% <ø> (ø)
include/boost/beast/websocket/impl/ping.hpp 95.61% <0.00%> (+0.87%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90e37ae...2d7b5b5. Read the comment docs.

@sehe sehe marked this pull request as draft May 2, 2022 12:53
@alandefreitas
Copy link
Member

alandefreitas commented May 2, 2022

Mmm... So the observable differences for users had no impact on the current implementation, right?

I just had a look at occurrences of max_size(), remove_suffix(), and remove_prefix(), and there are no differences that would impact beast.

@sehe
Copy link
Collaborator Author

sehe commented May 2, 2022

@alandefreitas Correct, which is why I went ahead and created the PR and suggested release notes to detail any external observabilities that users might encounter in their own code using Beast.

@sehe sehe marked this pull request as ready for review May 8, 2022 15:56
@sehe
Copy link
Collaborator Author

sehe commented May 8, 2022

@vinniefalco marked as ready for review since the hash_value support is in Core. If you think we should go the deprecation warning/conditional compilation route instead, let me know.

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://2418.beastdocs.prtest.cppalliance.org/libs/beast/doc/html/index.html

@vinniefalco
Copy link
Member

I would just change the commit message to

Use core string_view:

fix #2417

This improves inter-conversion...

@sehe
Copy link
Collaborator Author

sehe commented May 8, 2022

Also, turns out I was just copy pasting the bullet list formatting from a place in the release-notes where it was /also/ not rendering correctly. :derp: Hopefully now it comes out readable.

@vinniefalco
Copy link
Member

Yep that's why we have doc previews :) (Thanks Sam!)
https://2418.beastdocs.prtest.cppalliance.org/libs/beast/doc/html/beast/release_notes.html

@cppalliance-bot
Copy link

pullrequest
Timeline tracing charts: https://2418.beast.tracing.cppalliance.org/index.html

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://2418.beastdocs.prtest.cppalliance.org/libs/beast/doc/html/index.html

@cppalliance-bot
Copy link

pullrequest
Timeline tracing charts: https://2418.beast.tracing.cppalliance.org/index.html

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
@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://2418.beastdocs.prtest.cppalliance.org/libs/beast/doc/html/index.html

@sehe
Copy link
Collaborator Author

sehe commented May 9, 2022

Finally fixed the release notes. Took the liberty to fix some of the prior broken lists as well. It took me setting up local automatic doc generation, but now we have something :)

@cppalliance-bot
Copy link

pullrequest
Timeline tracing charts: https://2418.beast.tracing.cppalliance.org/index.html

@vinniefalco
Copy link
Member

Please merge this if it is ready

@sehe sehe merged commit 9d23bec into boostorg:develop May 10, 2022
@sehe sehe deleted the issue2417 branch June 8, 2022 23:30
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

Successfully merging this pull request may close these issues.

4 participants