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

Cannot use std::format on nholman::json objects #4261

Open
2 tasks
AA1999 opened this issue Jan 4, 2024 · 9 comments
Open
2 tasks

Cannot use std::format on nholman::json objects #4261

AA1999 opened this issue Jan 4, 2024 · 9 comments
Labels
kind: bug state: please discuss please discuss the issue or vote for your favorite option

Comments

@AA1999
Copy link

AA1999 commented Jan 4, 2024

Description

I've opened a json file and parsed it with this library and I wrote this:

std::string connection_config = std::format("host={} port={} dbname={} user={} password='{}'", db_config.at("host"),
												db_config.at("port"), db_config.at("dbname"), db_config.at("user"),
												db_config.at("password"));

However I get this error:

In template: call to deleted constructor of 'typename format_context::formatter_type<basic_json<std::map, std::vector, string, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, vector<unsigned char, allocator<unsigned char>>>>' (aka 'formatter<nlohmann::basic_json<std::map, std::vector, std::string, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char>>>, char>')

Note of interest: this only happens with std::format, fmt::format is fine

Reproduction steps

Open a file with std::ifstream
Pass the file object to nlohmann::json
Parse the json file
Try to format it using std::format

Expected vs. actual results

Expected result: The json array being formatted properly
Actual result: error

Minimal code example

auto db_config_file = std::ifstream {"~/.reacatio/database.json"};

	json db_config = json::parse(db_config_file);
	db_config_file.close();

	std::string connection_config = std::format("host={} port={} dbname={} user={} password='{}'", db_config.at("host"),
												db_config.at("port"), db_config.at("dbname"), db_config.at("user"),
												db_config.at("password"));

Error messages

In template: call to deleted constructor of 'typename format_context::formatter_type<basic_json<std::map, std::vector, string, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, vector<unsigned char, allocator<unsigned char>>>>' (aka 'formatter<nlohmann::basic_json<std::map, std::vector, std::string, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char>>>, char>')

Compiler and operating system

clang 17, linux

Library version

3.11.2

Validation

@gregmarr
Copy link
Contributor

gregmarr commented Jan 4, 2024

User-defined types must opt in to being formatted by std::format by specializing std::formatter. https://en.cppreference.com/w/cpp/utility/format/formatter

fmt::format is probably picking up implicit conversions, which means that it can get them very very wrong. What happens if you have a json file with numbers instead of strings? Does fmt::format display them properly? I would be very surprised, unless someone added support for this specific library.

I assume that std::format explicitly excludes implicit conversions to types that std::formatter is enabled for, which is much safer.

@AA1999
Copy link
Author

AA1999 commented Jan 4, 2024

For some reason now fmt is also now not working (I'm pretty sure it did before, maybe an earlier version)

In file included from test.cpp:4:
In file included from /usr/include/fmt/format.h:49:
/usr/include/fmt/core.h:2548:45: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<nlohmann::basic_json<>, char>'
    type_is_unformattable_for<T, char_type> _;
                                            ^
/usr/include/fmt/core.h:2611:23: note: in instantiation of function template specialization 'fmt::detail::parse_format_specs<nlohmann::basic_json<>, fmt::detail::compile_parse_context<char>>' requested here
        parse_funcs_{&parse_format_specs<Args, parse_context_type>...} {}
                      ^
/usr/include/fmt/core.h:2740:47: note: in instantiation of member function 'fmt::detail::format_string_checker<char, nlohmann::basic_json<>, nlohmann::basic_json<>>::format_string_checker' requested here
      detail::parse_format_string<true>(str_, checker(s));
                                              ^
test.cpp:9:27: note: in instantiation of function template specialization 'fmt::basic_format_string<char, nlohmann::basic_json<> &, nlohmann::basic_json<> &>::basic_format_string<char[6], 0>' requested here
        std::cout << fmt::format("{} {}", data.at("host"), data.at("port"));
                                 ^
/usr/include/fmt/core.h:1554:45: note: template is declared here
template <typename T, typename Char> struct type_is_unformattable_for;
                                            ^
test.cpp:9:27: error: call to consteval function 'fmt::basic_format_string<char, nlohmann::basic_json<> &, nlohmann::basic_json<> &>::basic_format_string<char[6], 0>' is not a constant expression
        std::cout << fmt::format("{} {}", data.at("host"), data.at("port"));
                                 ^
In file included from test.cpp:4:
In file included from /usr/include/fmt/format.h:49:
/usr/include/fmt/core.h:1576:63: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<nlohmann::basic_json<>, char>'
    type_is_unformattable_for<T, typename Context::char_type> _;
                                                              ^
/usr/include/fmt/core.h:1808:23: note: in instantiation of function template specialization 'fmt::detail::make_arg<true, fmt::basic_format_context<fmt::appender, char>, nlohmann::basic_json<>, 0>' requested here
        data_{detail::make_arg<is_packed, Context>(args)...} {
                      ^
/usr/include/fmt/core.h:1826:10: note: in instantiation of function template specialization 'fmt::format_arg_store<fmt::basic_format_context<fmt::appender, char>, nlohmann::basic_json<>, nlohmann::basic_json<>>::format_arg_store<nlohmann::basic_json<>, nlohmann::basic_json<>>' requested here
  return {args...};
         ^
/usr/include/fmt/core.h:2788:28: note: in instantiation of function template specialization 'fmt::make_format_args<fmt::basic_format_context<fmt::appender, char>, nlohmann::basic_json<>, nlohmann::basic_json<>>' requested here
  return vformat(fmt, fmt::make_format_args(args...));
                           ^
test.cpp:9:20: note: in instantiation of function template specialization 'fmt::format<nlohmann::basic_json<> &, nlohmann::basic_json<> &>' requested here
        std::cout << fmt::format("{} {}", data.at("host"), data.at("port"));
                          ^
/usr/include/fmt/core.h:1554:45: note: template is declared here
template <typename T, typename Char> struct type_is_unformattable_for;
                                            ^
/usr/include/fmt/core.h:1579:3: error: static assertion failed due to requirement 'formattable': Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
  static_assert(
  ^
4 errors generated.

Anyhow then ig nlohmann::json would need a std::formatter?

@gregmarr
Copy link
Contributor

gregmarr commented Jan 4, 2024

Anyhow then ig nlohmann::json would need a std::formatter?

Yes. I assume, but can't say for sure, that a PR would be welcome as long as it was sufficient protected such that it didn't affect the ability to build as C++11. There are other features that are conditionally available based on the C++ standard in use. @nlohmann would have the final say on that.

@AA1999
Copy link
Author

AA1999 commented Jan 4, 2024

As far as I'm aware there are micros to check C++ version and not define a function if it's not matching, so maybe this could be added only if c++20 flag is added.

@gregmarr
Copy link
Contributor

gregmarr commented Jan 4, 2024

Yes, this library has JSON_HAS_CPP_20 for that.

@heinemml
Copy link
Contributor

heinemml commented Mar 6, 2024

I ran into this when updating dependencies.

For fmt you can observe this break in this godbolt example: https://godbolt.org/z/jx7fTen7Y

with fmt 9.1.0 it works, with 10.0.0 it still compiles but throws and with 10.1.1 it finally doesn't compile anymore.

I guess something like this would fix the issue for std::format

#if defined(JSON_HAS_CPP_20) && __has_include(<format>)

#include <format>

template <>
struct std::formatter<json> {

    constexpr auto parse(std::format_parse_context& ctx) {
        return ctx.begin();
    }

    auto format(const json& j, std::format_context& ctx) const {
        return std::format_to(ctx.out(), "{}", to_string(j));
    }
};

#endif

but this doesn't fix it for fmt :(.

@heinemml
Copy link
Contributor

heinemml commented Mar 7, 2024

fmtlib/fmt#3648

@heinemml
Copy link
Contributor

heinemml commented Mar 7, 2024

To implement a formatter which would work from fmt 10.0.0 onwards it would suffice to implement format_as as follows:

namespace nlohmann {

auto format_as(const json& j) { return j.dump(); }

}

Godbolt: https://godbolt.org/z/vEEad83h3

@nlohmann
Copy link
Owner

Alright, I finally had some time to look into this. If I understand this correctly, there are two issues:

  1. How to use the library with std::format
  2. How to use the library with fmtlib

For the first, a specialization of std::formatter is needed. This would mean we would need to add something like

template<>
struct std::formatter<nlohmann::json, char>
{
    bool pretty = false;

    template<class ParseContext>
    constexpr ParseContext::iterator parse(ParseContext& ctx)
    {
        auto it = ctx.begin();
        if (it == ctx.end())
            return it;

        if (*it == '#')  // Use `#` to indicate pretty formatting
        {
            pretty = true;
            ++it;
        }
        if (it != ctx.end() && *it != '}')
            throw std::format_error("Invalid format args for nlohmann::json.");

        return it;
    }

    template<class FmtContext>
    FmtContext::iterator format(const nlohmann::json& j, FmtContext& ctx) const
    {
        std::ostringstream out;
        if (pretty)
            out << std::setw(4) << j;
        else
            out << j;  // Compact JSON

        return std::ranges::copy(out.str(), ctx.out()).out;
    }
};

Of course, guarded with the C++20 macro and proper library includes.

This code works with std::format, but using it with fmt::print triggers an assertion. I have not looked into this whether it's a bug on the library side.

So for the second point, adding the code from #4261 (comment) does the job.

It would feel much better to only have one of the snippets work with both libraries. Furthermore, we may need to guard the code with another macro like JSON_STD_FORMAT_SUPPORT (defined by default) in case client code already provided their own specialization.

What do you think?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

4 participants