fix: un-qualify Range::value_type for compatibility with fmt v11 #2265
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yfeldblum
reviewed
Jul 24, 2024
`folly::StringPiece` no longer works with fmt v11 due to `value_type` being `const char` instead of the `char` expected by fmt. Looking at `std::span`, which is probably the closest equivalent to `folly::Range`, `value_type` is defined as `std::remove_cv_t<T>`, so I think it would make sense for `folly::Range` to define `value_type` similarly, i.e. without cv-qualification. This change removes cv-qualifiers from `value_type` and replaces the the use of `value_type&` by `reference` and `const value_type&` by the newly introduced `const_reference`.
mhx
force-pushed
the
mhx/range-unqualify-value-type
branch
from
b2b311d
to
d00d4a6
Compare
|
@vitaut has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Thanks! I'll add this to the Fedora build while you work with @yfeldblum to get this merged upstream |
7 tasks
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 26, 2024
Summary:
Currently, `UTF8StringPiece` is defined in terms of `Range<boost::u8_to_u32_iterator<...>>`. But this makes certain necessary changes in `Range` fail to compile. Change `UTF8StringPiece` to be a standalone range type.
Unblocks {D60181556} (aka #2265), which intends to resolve a problem blocking `folly::StringPiece` from being formattable with `fmt::format` since `fmt-11`.
Reviewed By: vitaut
Differential Revision: D60252703
fbshipit-source-id: 282d166c1456bdc5e311f4ed7c3ce8eae91422b0
|
Superseded by 21e8dcd but thanks for working on this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
folly::StringPieceno longer works with fmt v11 due tovalue_typebeingconst charinstead of thecharexpected by fmt.Looking at
std::span, which is probably the closest equivalent tofolly::Range,value_typeis defined asstd::remove_cv_t<T>, so I think it would make sense forfolly::Rangeto definevalue_typesimilarly, i.e. without cv-qualification.This change removes cv-qualifiers from
value_typeand replaces the the use ofvalue_type&byreferenceandconst value_type&by the newly introducedconst_reference.See fmtlib/fmt#4086 for additional context.