search

fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
19.83k stars 2.42k forks source link

Non-arithmetic format_as is not supported with fmt/base.h #4014

Closed jeremy-rifkin closed 1 day ago

jeremy-rifkin commented 3 weeks ago

3959 suggests that format_as should be supported with just fmt/core.h. I have hit a very surprising behavior where format_as returning a non-arithmetic type is completely broken unless including fmt/format.h: https://godbolt.org/z/344bznojf.

I see fmt/core.h is now just an alias for fmt/format.h on master so this should not reproduce currently, however, I want to flag this behavior in case it is unintentional and in case there are 10.x patch releases before the current core.h change is released.

vitaut commented 3 weeks ago

Closing since it doesn't reproduce on trunk and there are no plans for more 10.x releases but thanks for reporting.

jagoly commented 6 days ago

This seems to still be the case on trunk with base.h instead of core.h. format_as(Thing) -> int works with just base.h, but format_as(Thing) -> fmt::string_view or format_as(Thing) -> const char* requires format.h.

example: https://godbolt.org/z/5f75q8hh3

jeremy-rifkin commented 6 days ago

It’s possible CE’s trunk version of fmt is outdated

vitaut commented 6 days ago

Thanks for the repro, @jagoly, now I see the problem. The fix would be to move this formatter https://github.com/fmtlib/fmt/blob/b61c8c3d23b7e6fdf9d44593877dba1c8a291be1/include/fmt/format.h#L3970-L3971 to fmt/base.h. A PR would be welcome.

jagoly commented 2 days ago

It seems simply moving that over isn't enough, that just gives errors in base::format(val, ctx); because native_formatter::format is only declared in base.h, with the definition in format.h.

I believe the correct solution is to tweak https://github.com/fmtlib/fmt/blob/c4f6fa71357b223b0ab8ac29577c6228fde8853d/include/fmt/base.h#L1502 so that it can map any types where format_as returns a natively supported type. I don't see why mapping to a string view would be an issue, lifetime extension should keep the original object around until the end of the format call anyway? Maybe it has issues with dynamic argument stores?

I've tried to get this working, though when adding tests to test-base.cc it seems that its_a_trap breaks when adding any format_as overloads to the file, whether they return a mappable type or not. I could work around this by adding a new test file just for format_as, but it's probably best if someone with a bit more familiarity looks at it.

vitaut commented 1 day ago

It seems simply moving that over isn't enough ...

You are right. Unfortunately doing this for user-defined types would be unsafe and lifetime extension won't apply.

In any case, since includes in fmt/format.h have been optimized there is little value in trying to optimize this so I'm closing the issue.