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

Add support for __float128 or std::float128_t #3494

Closed
Eisenwave opened this issue Jun 16, 2023 · 12 comments
Closed

Add support for __float128 or std::float128_t #3494

Eisenwave opened this issue Jun 16, 2023 · 12 comments

Comments

@Eisenwave
Copy link

fmt 9 added support for fmt::to_string(__float128) but no formatter yet as far as I know. I've tried to make my own like below, but this failed:

#include <fmt/core.h>
#include <fmt/std.h>

template <>
struct fmt::formatter<__float128> : fmt::formatter<std::string> {
    template <typename FormatContext>
    auto format(__float128 number, FormatContext& ctx) const {
        return fmt::formatter<std::string>::format(fmt::to_string(number), ctx);
    }
};

int main() {
    fmt::format("{}", __float128{1.3});
}

Am I doing something wrong or is this a bug?

/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h: In substitution of 'template<class T, class Context> using mapped_type_constant = fmt::v10::detail::type_constant<decltype (fmt::v10::detail::arg_mapper<Context>().map(declval<const T&>())), typename Context::char_type> [with T = __float128; Context = fmt::v10::basic_format_context<fmt::v10::appender, char>]':
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:2588:66:   required from 'constexpr fmt::v10::detail::format_string_checker<Char, Args>::format_string_checker(fmt::v10::basic_string_view<Char>) [with Char = char; Args = {__float128}]'
<source>:13:16:   required from here
<source>:13:16:   in 'constexpr' expansion of 'fmt::v10::basic_format_string<char, __float128>("{}")'
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:1475:53: error: call of overloaded 'map(const __float128&)' is ambiguous
 1475 |     type_constant<decltype(arg_mapper<Context>().map(std::declval<const T&>())),
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
...

The error is very long, so I won't post all of it here. See https://godbolt.org/z/9GPM5MGvG for full output.

@vitaut
Copy link
Contributor

vitaut commented Jun 21, 2023

It is neither a bug nor you are doing anything wrong. {fmt} simply doesn't support formatting of __float128 (yet).

@vitaut vitaut changed the title Cannot create custom formatter for __float128 or std::float128_t Add support for __float128 or std::float128_t Jun 21, 2023
@Eisenwave
Copy link
Author

It is neither a bug nor you are doing anything wrong. {fmt} simply doesn't support formatting of __float128 (yet).

I can see that, but how come it also doesn't allow creating a user-defined formatter? Not having a formatter is a missing feature; making it impossible to create one is a bug.

@vitaut
Copy link
Contributor

vitaut commented Jun 23, 2023

You shouldn't be creating formatters for types you don't own and in this case it is intentionally impossible. You can create a formatter for a wrapper type though.

@janup2442
Copy link

Can you explain , what do you want in output ?? With a simple example , so that I can try to figure out . @Eisenwave

@Eisenwave
Copy link
Author

Can you explain , what do you want in output ?? With a simple example , so that I can try to figure out . @Eisenwave

fmt::format("{}", __float128{1.3}) should be equivalent to fmt::to_string(__float128{1.3}) or at least be similar in output. I'm not sure what you're confused about, as there is really only one possible implementation here.

It should have the same formatting option as floating-point numbers of lower precision.

@vitaut
Copy link
Contributor

vitaut commented Mar 22, 2024

__float128 support has been added in 5d63e87.

@vitaut vitaut closed this as completed Mar 22, 2024
@sketch34
Copy link

You shouldn't be creating formatters for types you don't own

@vitaut can you please explain this statement some more? There's guidance elsewhere suggesting people write custom formatters for std:: types, I'm a bit confused.

@vitaut
Copy link
Contributor

vitaut commented Jun 26, 2024

If you define a formatter for a third-party type you run into a risk of conflicts with someone doing the same. It is particularly bad for standard types since the newer version of the standard can add its own formatter causing breakages. So that guidance is wrong and I recommend ignoring it or better asking the authors to change/remove it.

@sketch34
Copy link

sketch34 commented Jun 27, 2024

since the newer version of the standard can add its own formatter causing breakages.

Okay so I guess fmt detects when some std:: type T has a standard library implementation of std::formatter<T> and tries to use that, but if we have also defined fmt::formatter<T> then things can go bad when we compile against a new stdlib.

@vitaut
Copy link
Contributor

vitaut commented Jun 27, 2024

To clarify: {fmt} doesn't use standard formatters but both the standard library and {fmt} can provide formatters for standard library and built-in types which will have similar effect.

@sketch34
Copy link

sketch34 commented Jul 2, 2024

{fmt} doesn't use standard formatters

Sorry I'm still not able to grasp what the issue with this is or how "the newer version of the standard can add its own formatter causing breakages". If fmt lib doesn't use standard formatters, how would these breakages occur?

If I write a fmt::formatter<std::float128_t> for my private project, what's the problem? The only thing I can think of is that if I upgrade to a new version of fmt lib that has support for std::float128_t then I'll get an ambiguous overload error, delete my formatter and move on. 🤔

@vitaut
Copy link
Contributor

vitaut commented Jul 5, 2024

If you do it in your own private project then it's less of a problem as you are the only one who will suffer because of the breakage.

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

4 participants