Skip to content

Commit

Permalink
Trying to improve errors in the unformattable case (#3478)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brevzin
brevzin authored Jul 1, 2023
1 parent e4c8cfe commit de4705f
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions include/fmt/core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,11 @@ constexpr auto encode_types() -> unsigned long long {
(encode_types<Context, Args...>() << packed_arg_bits);
}

#if defined(__cpp_if_constexpr)
// This type is intentionally undefined, only used for errors
template <typename T, typename Char> struct type_is_unformattable_for;
#endif

template <bool PACKED, typename Context, typename T, FMT_ENABLE_IF(PACKED)>
FMT_CONSTEXPR FMT_INLINE auto make_arg(T& val) -> value<Context> {
using arg_type = remove_cvref_t<decltype(arg_mapper<Context>().map(val))>;
Expand All @@ -1565,6 +1570,11 @@ FMT_CONSTEXPR FMT_INLINE auto make_arg(T& val) -> value<Context> {
"Formatting of non-void pointers is disallowed.");

constexpr bool formattable = !std::is_same<arg_type, unformattable>::value;
#if defined(__cpp_if_constexpr)
if constexpr (!formattable) {
type_is_unformattable_for<T, typename Context::char_type> _;
}
#endif
static_assert(
formattable,
"Cannot format an argument. To make type T formattable provide a "
Expand Down Expand Up @@ -2529,7 +2539,17 @@ FMT_CONSTEXPR auto parse_format_specs(ParseContext& ctx)
mapped_type_constant<T, context>::value != type::custom_type,
decltype(arg_mapper<context>().map(std::declval<const T&>())),
typename strip_named_arg<T>::type>;
#if defined(__cpp_if_constexpr)
if constexpr (std::is_default_constructible_v<
formatter<mapped_type, char_type>>) {
return formatter<mapped_type, char_type>().parse(ctx);
} else {
type_is_unformattable_for<T, char_type> _;
return ctx.begin();
}
#else
return formatter<mapped_type, char_type>().parse(ctx);
#endif
}

// Checks char specs and returns true iff the presentation type is char-like.
Expand Down

2 comments on commit de4705f

@GJanos
Copy link

@GJanos GJanos commented on de4705f Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @brevzin and fmt developer team.
I am working on a logging library in C++, I built my implementation upon Glaze C++ logging library, that uses the Format library. I had a working code, but I think, after refreshing my cmakelists file, which has glaze, and quill (quill has fmt) as fetchcontent, my quill and fmt related macros stopped working. I checked the repositories commit history, and seen some changes inside the fmt library that might affected something on my end. I am talking about the last 10 commits, or around that. I am saying this because I downloaded one of my older working commits from git, and was not able to run that either.

LOG_INFO(logger_bar, "{:f}",Trade_data(1, "BTC", 25432.23, 1.23, "C"));

This is the line that gives the errors. For my own type to be loggable I followed quills and fmts instructions and made changes accordingly (glz => copyable object, fmt => specialized formatter):

#include "log_data_info.h"

#include <glaze/glaze.hpp>
#include <iostream>
#include <utility>
#include "quill/Quill.h"
#include "quill/Utility.h"
#include <fmt/core.h>


class Trade_data {
public:
    Trade_data() = default;

    Trade_data(int id,
               std::string name,
               double price,
               double quantity,
               std::string exchange_name);

    Trade_data(const Trade_data& other);
    Trade_data& operator=(const Trade_data& other);
    Trade_data(Trade_data&& other) noexcept;
    Trade_data& operator=(Trade_data&& other) noexcept;

    static std::string serialize_best(const Trade_data &trade_data);
    static Trade_data deserialize_best(const std::string &trade_data_str);

    static Trade_data create_rand_trade_data();
    Trade_data create_price_movements();

public:
    int id;
    std::string name;
    double price;
    double quantity;
    std::string exchange_name;
};

// to be able to serialize Trade_data with glaze
template<>
struct glz::meta<Trade_data> {
    using T = Trade_data;
    static constexpr auto value = glz::object(
            "id", &T::id,
            "name", &T::name,
            "price", &T::price,
            "quantity", &T::quantity,
            "exchange_name", &T::exchange_name);
};

// to be able to log Trade_data with quill
namespace quill {
    template<>
    struct copy_loggable<Trade_data> : std::true_type {
    };
}

template<>
struct fmt::formatter<Trade_data> {
    char presentation = 'f';

    constexpr auto parse(format_parse_context &ctx) -> format_parse_context::iterator {
        auto it = ctx.begin(), end = ctx.end();
        if (it != end && (*it == 'f' || *it == 'e')) presentation = *it++;
        return it;
    }

    auto format(const Trade_data &trade_data, format_context &ctx) const -> format_context::iterator {
        return fmt::format_to(ctx.out(),
                              "{}{}{}",
                              typeid(Trade_data).name(),
                              TYPE_SEPARATOR,
                              glz::write_json(trade_data));
    }
};

This alone, was enough for me previously to log my own Trade_data objects without any problems, but right now I get a lot of errors.
The error messages seem to be related to quill, but the c++ quill library have not had a commit since 2 weeks, so that is why I assume it might be related to fmt, but I will ask quill as well.
Thank you for your time!

@vitaut
Copy link
Contributor

@vitaut vitaut commented on de4705f Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works with the {fmt} master (https://godbolt.org/z/Troxqzz5b) so the issue is likely elsewhere. I am not familiar with quill unfortunately.

Please sign in to comment.