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

<format>: Conditionally emit special diagnostic message for lack of const #4461

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

Fixes #4202.

Also adds static_assert messages in the constructor body of basic_format_arg::handle, and moves the static_asserts into the lambda body.

Currently, Clang emits 2 static_assert failures for such an error, one in make_(w)format_args, one in the handle ctor. Because lack of formatter specialization or const on format can trigger static_assert failure in the constructor body of basic_format_arg::handle now.

This PR slightly worsens the error messages for Clang (2 similar static_assert messages will be emitted). However, it seems that cleaner error messages can be achieved by conditional compilation (skipping static_asserts in make_(w)format_args for Clang).

Also emit `static_assert` error messages for `basic_format_arg::handle`.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner March 9, 2024 13:58
@github-actions github-actions bot added this to Initial Review in Code Reviews Mar 9, 2024
@StephanTLavavej StephanTLavavej added enhancement Something can be improved format C++20/23 format labels Mar 9, 2024
@StephanTLavavej StephanTLavavej self-assigned this Mar 9, 2024
@@ -707,15 +715,23 @@ public:
using _Td = remove_const_t<_Ty>;
// doesn't drop const-qualifier per an unnumbered LWG issue
using _Tq = conditional_t<_Formattable_with<const _Ty, _Context>, const _Ty, _Ty>;
if constexpr (_Formattable_with_non_const<_Tq, _Context>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ctor is exposition-only per [format.arg]/10 but is public in MSVC STL...
Perhaps we should consider making it private (in another PR, together with non-Standard public ctors of basic_format_context).

stl/inc/format Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Mar 13, 2024
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in Code Reviews Mar 13, 2024
@StephanTLavavej StephanTLavavej self-assigned this Mar 15, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 33854e5 into microsoft:main Mar 16, 2024
35 checks passed
Code Reviews automation moved this from Ready To Merge to Done Mar 16, 2024
@frederick-vs-ja frederick-vs-ja deleted the formatter-non-const-error branch March 16, 2024 05:37
@StephanTLavavej
Copy link
Member

static_assert(true, "Thanks! 🎉😸☑️");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved format C++20/23 format
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<format>: Hideous compiler errors when formatter<UDT>::format() isn't const
2 participants