Fix FMT_COMPILE() with custom types
#2326
Merged
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
vitaut
requested changes
May 31, 2021
test/compile-test.cc
Outdated
Comment on lines
71
to
100
| struct spec_handler { | ||
| formatter& f; | ||
| basic_format_parse_context<char>& context; | ||
|
|
||
| void on_error(const char* msg) { FMT_THROW(format_error(msg)); } | ||
| FMT_CONSTEXPR void on_fill(basic_string_view<char> fill) { | ||
| f.specs.fill = fill; | ||
| } | ||
| FMT_CONSTEXPR void on_align(align_t align) { f.specs.align = align; } | ||
| FMT_CONSTEXPR void on_width(int width) { f.specs.width = width; } | ||
| FMT_CONSTEXPR void on_precision(int) {} | ||
| FMT_CONSTEXPR void end_precision() {} | ||
| template <typename Id> | ||
| FMT_CONSTEXPR detail::arg_ref<char> make_arg_ref(Id arg_id) { | ||
| context.check_arg_id(arg_id); | ||
| return detail::arg_ref<char>(arg_id); | ||
| } | ||
| FMT_CONSTEXPR detail::arg_ref<char> make_arg_ref( | ||
| basic_string_view<char> arg_id) { | ||
| context.check_arg_id(arg_id); | ||
| return detail::arg_ref<char>(arg_id); | ||
| } | ||
| FMT_CONSTEXPR detail::arg_ref<char> make_arg_ref(detail::auto_id) { | ||
| return detail::arg_ref<char>(context.next_arg_id()); | ||
| } | ||
| template <typename Id> FMT_CONSTEXPR void on_dynamic_width(Id arg_id) { | ||
| f.width_ref = make_arg_ref(arg_id); | ||
| } | ||
| template <typename Id> FMT_CONSTEXPR void on_dynamic_precision(Id) {} | ||
| }; |
There was a problem hiding this comment.
This is a bit of an overkill for the test. Let's do some basic parsing instead of relying on internal API.
There was a problem hiding this comment.
So, commits 1+2 then, dropping the 3
There was a problem hiding this comment.
Yeah, could you update the PR?
There was a problem hiding this comment.
Of course, I will do so, but maybe an hour later
Good catch, added to changelog in 9bb406d. |
alexezeder
force-pushed
the
fix/fmt_compile_with_custom_types
branch
from
cd72ce7 to
7bdc3fd
Compare
|
Thank you! |
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.
By reading the changelog for upcoming release, I realized that it does not mention one of the major breaking change of custom formatters for compile-time formatting -
const-qualifiedformatmethod, so I decided to check how do custom formatters work at compile-time and realized that they actually don't work.There are 3 commits, by the complexity of changes from the commit that just fixes the library, to the commit that adds dynamic width handling for the
test_formattableclass in compile-test.cc. The latter two can be removed or squashed into the first one, on demand. This time I cannot use custom formatters fromchrono.hbecause they usememory_bufferand it's not ready to be used at compile-time yet.