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

Regression in formatting a type through a pointer using specialization #3157

Closed
rburchell opened this issue Oct 31, 2022 · 3 comments
Closed
Labels

Comments

@rburchell
Copy link

It seems that after the fix for #2717 (8f8a1a0), it seems it is no longer possible to specialize formatter for a given pointer type:

error: static_assert failed due to requirement 'formattable_pointer' "Formatting of non-void pointers is disallowed."

Reproduced on godbolt here: https://godbolt.org/z/h3cWYYev8.

Though I notice that even just reverting that commit isn't quite enough, as the specialization doesn't seem to get used anymore. It certainly used to work in fmt 8.1.1 - I'm not quite sure when that broke.

Is this intentional? It seems like a pretty inconvenient behaviour change.

@vitaut
Copy link
Contributor

vitaut commented Nov 2, 2022

Formatting of non-void pointers is intentionally disallowed. If you want to format a pointer cast it to void* or use fmt::ptr. Otherwise define a formatter for the type itself and dereference when formatting. Some earlier versions didn't diagnose this properly.

@vitaut vitaut closed this as completed Nov 2, 2022
@vitaut vitaut added the question label Nov 2, 2022
@rburchell
Copy link
Author

@vitaut Can I ask why? It seems very unfriendly to shunt all the work of trying to safely dereference onto the individual fmt uses, rather than leaving it in one place (in the specialization).

Here's an example, which used to work just fine:

class MyClass
{
public:
    int thing = 42;
};

namespace fmt {

template <>
struct formatter<MyClass*>
{
    constexpr auto parse(format_parse_context& ctx)
    {
        return ctx.begin();
    }

    template <typename FormatContext>
    auto format(const MyClass* const value, FormatContext& ctx)
    {
        if (value) {
            return format_to(ctx.out(), "MyClass(this: {}, thing={})", (void*)value, value->thing);
        } else {
            return format_to(ctx.out(), "MyClass(this: nullptr)");
        }
    }
};

}

Test code (dbg is just my own light-weight wrapper to print using fmt, it's not really relevant):

    MyClass* m1 = nullptr;
    MyClass* m2 = new MyClass;
    dbg("test! {}", m1);
    dbg("test! {}", m2);

(Past) output:

test! MyClass(this: 0x0)
test! MyClass(this: 0x602000002630, thing=42)

If we do it your way, and dereference:

    dbg("test! {}", *m1);

... this will now immediately crash. It's easily fixed in this contrived case, sure, but imagine you're printing a tree node where you have multiple pointers to try output, that's going to get awkward pretty fast.

I suppose a benefit is that with a specialization on the pointer, you can't (easily?) use fmt without a pointer, but it was always trivial to get one:

MyClass m3;
dbg("test! {}", m3); // this won't compile
dbg("test! {}", &m3); // but this will

@vitaut
Copy link
Contributor

vitaut commented Nov 6, 2022

It was an early design decision based on the fact that pointer formatting has a separate semantics from value formatting but unfortunately there was a regression in enforcing it. Basically the same reasoning as discussed in https://accu.org/journals/overload/17/89/wilson_1539/. You can achieve similar behavior via an adapter type which is the usual extension mechanism in {fmt}. So the recommended solution is to define a formatter specialization for MyClass and an adapter for MyClass* or generic set of classes which will give usage similar to the following:

MyClass m3;
dbg("test! {}", m3);
dbg("test! {}", ptr(&m3));

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

No branches or pull requests

2 participants