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

fmtlib support #2780

Open
ni-ndose opened this issue Dec 11, 2023 · 4 comments
Open

fmtlib support #2780

ni-ndose opened this issue Dec 11, 2023 · 4 comments

Comments

@ni-ndose
Copy link

Description
We have a codebase whose data structures can be printed via custom fmtlib formatters. It'd be great if Catch could detect these formatters automatically and use them.

Additional context
I've started experimenting with introducing this into your codebase - but wanted to check if it's even a desired feature of catch2 before proceeding:

Naively, the easiest approach to this would be to copy what's done for operator<< in catch_tostring.hpp - in addition to [IsStreamInsertable] (

class IsStreamInsertable {
) used for streaming, we offer a "IsFmtFormattable" that calls fmt::is_formattable to see if something can be printed via fmtlib.

We could hook into __has_include to see if fmtlib is even present, and if not, ifdef all that stuff out.

@mattgodbolt
Copy link
Contributor

I'd love this too; we have tried a few times unsuccessfully to hack it ourselves

@horenmar
Copy link
Member

I am okay with this feature, but there are some things that need to be satisfactorily answered

  • Is this specifically for fmt::format, or for std::format as well? If both are present, which one to pick? In theory, C++ should slowly migrate towards std::format, but I can say that none of my own projects did, because fmtlib evolves faster and is generally better.
  • I don't think there is currently a way to support autodetection for fmtlib use-in-project, so it would need to be another configuration toggle. For <format>, we can automatically detect it being available through feature macros. (I do not want to use __has_include, because that returns true as long as fmt is somewhere in the include path, even if it is not packaged as part of the project. This can cause issues with e.g. vcpkg classic mode.)
  • What is the compile-time overhead of adding this?

@diablodale
Copy link

I recently added std::format support to Catch2 with no catch2 code change. Only need a concept and a StringMaker template.
The same approach could be added into Catch2. Or, in its default handling method, it could if constexpr within it to first check the formattable concept before choosing to call std::format or operator<<, etc. Or, perhaps concept requires the whole default StringMaker so it uses the std::formatter version for those types which satisfy the concept.

The concept below closely aligns with the c++23 std::formattable concept. I made the 2nd param char its default where I think the std doesn't. Easy to macro test for c++23 and alias to the std version.

namespace dp::concepts {

// helper
template <typename T, typename Context, typename Formatter = Context::template formatter_type<std::remove_const_t<T>>>
concept FormattableWithHelper = std::semiregular<Formatter> &&
    requires(Formatter& f, const Formatter& cf, T&& t, Context fc, std::basic_format_parse_context<typename Context::char_type> pc) {
        { f.parse(pc) } -> std::same_as<typename decltype(pc)::iterator>;
        { cf.format(t, fc) } -> std::same_as<typename Context::iterator>;
    };

// check if type is formattable with std::format
template <typename T, typename CharT = char>
concept Formattable = FormattableWithHelper<std::remove_reference_t<T>, std::basic_format_context<std::back_insert_iterator<std::basic_string<CharT>>, CharT>>;

}   // namespace dp::concepts

Then just need tiny template

#include <catch2/catch_test_macros.hpp>
#include <format>

// Catch2 StringMaker specialization
namespace Catch {
    template<dp::concepts::Formattable T>
    struct StringMaker<T> {
        static std::string convert(const T& t) {
            return std::format("{}", t);
        }
    };
}

@diablodale
Copy link

I can make a PR for native std::format support by adjusting src/catch2/catch_tostring.hpp, adding CATCH_CONFIG_ENABLE_FORMAT_STRINGMAKER, and some various bits like ...ALL_STRINGMAKERS, cmake, test cases, etc. I see std::enable_if_t to do some critical logic so I'll need to adjust something so that I can use c++20 concept alongside the older std::enable_if_t approach for compat.

I would need to know from the core Catch2 team their philosophy on stringify priorities. StringMaker has within it two choices, IsStreamInsertable = true or false. If true it operator<<, if false, it cascades to a series predefined specializations of StringMaker, project specializations of StringMaker, CATCH_CONFIG_FALLBACK_STRINGIFIER, etc. I would need to have clarity on where in this stringification order std::format should be.

I'm rather agnostic. I have noticed a general disdain of iostreams where std::format is available. This suggest to me that it should be before even operator<<.

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