Exclude recursive ranges from the formatter specialization for ranges#2974
Conversation
… conditionals for filesystem::path testing that does not run into the ambiguity problem.
…cursive ranges and reject them in formatter specialization for ranges. In addition, introduce additional wrapper traits for the individual logical operands of the complete range constraints
…ter specialization for std::filesystem::path
…or the formatter specialization for std::filesystem::path
include/fmt/ranges.h
Outdated
| conditional_t<has_const_begin_end<R>::value, const R, R>; | ||
|
|
||
| template <typename R> | ||
| struct is_not_recursive_range : bool_constant< |
There was a problem hiding this comment.
is_not_recursive_range -> is_nonrecursive_range
There was a problem hiding this comment.
Renamed as suggested.
include/fmt/ranges.h
Outdated
| template <typename R, typename Char> | ||
| struct is_formattable_delayed : is_formattable< | ||
| detail::uncvref_type<detail::maybe_const_range<R>>, Char> { | ||
| }; | ||
|
|
||
| template <typename R, typename Char> | ||
| struct has_fallback_formatter_delayed : detail::has_fallback_formatter< | ||
| detail::uncvref_type<detail::maybe_const_range<R>>, Char> { | ||
| }; |
There was a problem hiding this comment.
What are these for? I'd suggest keeping them inlined as before.
There was a problem hiding this comment.
The whole point of this P/R is to ensure that the individual parts of the condition are evaluated in short-circuit manner. In C++ without language concepts this is only possible by defining type predicates that get only evaluated if
8000
the corresponding predicate template class definition becomes instantiated. Due to std::conjunction and friends we therefore need individual class templates. The trailing delayed is intended to emphasize their job.
There was a problem hiding this comment.
Could you clarify this in a comment?
There was a problem hiding this comment.
I'd suggest either adding a generic thing to delay instantiation, like so:
template <template <class...> class F, typename... Args> struct delayed : F<Args...> { };to be used as:
disjunction<
delayed<is_formattable, detail::uncvref_type<detail::maybe_const_range<R>>, Char>,
delayed<detail::has_fallback_formatter, detail::uncvref_type<detail::maybe_const_range<R>>, Char>
>or, if we're going to manually do our own thing, do all of it together:
template <class T, class Char>
struct delayed_fallback_formattable
: disjunction<
is_formattable<T, Char>,
detail::has_fallback_formatter<T, Char>>
{ };to be used as:
conjunction<
A,
B,
delayed_fallback_formattable<detail::uncvref_type<detail::maybe_const_range<R>>, Char>
>Put differently - instead of writing two bespoke delayed traits, write either one generic delayer or one bespoke delayed trait.
There was a problem hiding this comment.
This is a good suggestion but note that has_fallback_formatter will soon go away so I suggest keeping two bespoke traits. Keeping both will simplify removing has_fallback_formatter_delayed later.
There was a problem hiding this comment.
Oh. Ok that makes sense then. When you say soon, how soon?
There was a problem hiding this comment.
I merged now has_fallback_formatter_delayed into is_formattable_delayed and got rid of the former.
There was a problem hiding this comment.
how soon?
In the next major release about a year from now.
include/fmt/ranges.h
Outdated
|
|
||
| template <typename R> | ||
| struct is_not_recursive_range : bool_constant< | ||
| !std::is_same<detail::uncvref_type<R>, R>::value> {}; |
There was a problem hiding this comment.
Contrary to the name of this trait it only checks recursiveness but not whether R is a range. I suggest adding is_range check here.
There was a problem hiding this comment.
Again, we want to prevent instantiation if the is_recursive predicate unless the is_range predicate has evaluated to true. See above. We could rename it to something different, such as is_not_recursive, if you prefer and add a comment that says that we assume here that is_range is evaluated to true.
…ent that explains it being depending on is_range being true - Merge has_fallback_formatter_delayed into is_formattable_delayed and add comment that explains it being depending on is_not_recursive_range being true - Replace disjunction in formatter specialization by has_fallback_formatter_delayed - Get rid of unneeded detail:: prefixes within namespace detail
|
Merged, thanks! |
No description provided.