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

Broken fmt::is_formattable<void> #4147

Closed
phprus opened this issue Sep 3, 2024 · 9 comments
Closed

Broken fmt::is_formattable<void> #4147

phprus opened this issue Sep 3, 2024 · 9 comments

Comments

@phprus
Copy link
Contributor

phprus commented Sep 3, 2024

Expect:

fmt::is_formattable<void>::value == false

Actual (build error):

In file included from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:41,
                 from <source>:1:
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/base.h: In substitution of 'template<class T, class Char> using is_formattable = fmt::v11::bool_constant<(! std::is_base_of<fmt::v11::detail::unformattable, decltype (fmt::v11::detail::arg_mapper<Char>::map(declval<T&>()))>::value)> [with T = void; Char = char]':
<source>:6:44:   required from here
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/base.h:1269:73: error: forming reference to void
 1269 | using mapped_t = decltype(detail::arg_mapper<Char>::map(std::declval<T&>()));
      |                                                         ~~~~~~~~~~~~~~~~^~
Execution build compiler returned: 1

godbolt (new link):
https://godbolt.org/z/xro8619b4

@vitaut
Copy link
Contributor

vitaut commented Sep 4, 2024

I think it's fine for is_formattable to give a compile-time error for void because it's not something that can be passed as an argument to a formatting function.

@vitaut vitaut closed this as completed Sep 4, 2024
@vitaut vitaut added the question label Sep 4, 2024
@phprus
Copy link
Contributor Author

phprus commented Sep 4, 2024

But a compile-time error breaks a possible enable_if condition like std::is_void<T>::value || is_formattable<T, Char>::value.
See: #4148

@Dani-Hub
Copy link
Contributor

Dani-Hub commented Sep 4, 2024

But a compile-time error breaks a possible enable_if condition like std::is_void<T>::value || is_formattable<T, Char>::value. See: #4148

A possible workaround in this situation is to replace the || by disjunction_v (Untested - that assumes that is_formattable is suitable as such a predicate).

@phprus
Copy link
Contributor Author

phprus commented Sep 4, 2024

A possible workaround in this situation is to replace the || by disjunction_v (Untested - that assumes that is_formattable is suitable as such a predicate).

No, this is not working
https://godbolt.org/z/jvqja873h

@vitaut
Copy link
Contributor

vitaut commented Sep 4, 2024

Thinking more of it, other traits do work with void so let's make is_formattable work with it too.

@vitaut vitaut reopened this Sep 4, 2024
@vitaut
Copy link
Contributor

vitaut commented Sep 4, 2024

Done in 3df47a4. Thanks for the suggestion.

@vitaut vitaut closed this as completed Sep 4, 2024
@Dani-Hub
Copy link
Contributor

Dani-Hub commented Sep 4, 2024

Done in 3df47a4. Thanks for the suggestion.

What about using std::is_void instead of std::is_same<void, ...>? This would also work for cv void.

@vitaut
Copy link
Contributor

vitaut commented Sep 4, 2024

What about using std::is_void instead of std::is_same<void, ...>? This would also work for cv void.

A PR would be welcome.

@phprus
Copy link
Contributor Author

phprus commented Sep 4, 2024

A PR would be welcome.

I made this change in a separate commit in PR #4148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants