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

broken operator<< support in 8.x #2759

Closed
andijcr opened this issue Feb 11, 2022 · 5 comments
Closed

broken operator<< support in 8.x #2759

andijcr opened this issue Feb 11, 2022 · 5 comments
Labels

Comments

@andijcr
Copy link

andijcr commented Feb 11, 2022

https://gcc.godbolt.org/z/bfvGf8bb4

#include <ostream>

#include "fmt/ostream.h"
#include "fmt/printf.h"

struct to_format {
    friend std::ostream& operator<<(std::ostream& os, to_format const& tf) {
        return os << "formated :" << uintptr_t(&tf);
    }
};

int main() { fmt::print("{}\n", to_format{}); } 

it seems that fmt v7 could use friend operator<< as a fallback formatter, while with v8 it fails with

<source>:11:46: warning: ignoring return value of 'std::string fmt::v8::format(fmt::v8::format_string<T ...>, T&& ...) [with T = {to_format}; std::string = std::__cxx11::basic_string<char>; fmt::v8::format_string<T ...> = fmt::v8::basic_format_string<char, to_format>]', declared with attribute 'nodiscard' [-Wunused-result]
   11 | int main() { fmt::format("{}\n", to_format{}); }
      |                                              ^
In file included from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:50,
                 from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/ostream.h:13,
                 from <source>:3:
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:3101:31: note: declared here
 3101 | FMT_NODISCARD FMT_INLINE auto format(format_string<T...> fmt, T&&... args)
      |                               ^~~~~~
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h: In instantiation of 'constexpr fmt::v8::detail::value<Context> fmt::v8::detail::make_arg(T&&) [with bool IS_PACKED = true; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; fmt::v8::detail::type <anonymous> = fmt::v8::detail::type::custom_type; T = to_format&; typename std::enable_if<IS_PACKED, int>::type <anonymous> = 0]':
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:1834:77:   required from 'constexpr fmt::v8::format_arg_store<Context, Args>::format_arg_store(T&& ...) [with T = {to_format&}; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; Args = {to_format}]'
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:1851:38:   required from 'constexpr fmt::v8::format_arg_store<Context, typename std::remove_cv<typename std::remove_reference<Args>::type>::type ...> fmt::v8::make_format_args(Args&& ...) [with Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; Args = {to_format&}]'
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:3103:44:   required from 'std::string fmt::v8::format(fmt::v8::format_string<T ...>, T&& ...) [with T = {to_format}; std::string = std::__cxx11::basic_string<char>; fmt::v8::format_string<T ...> = fmt::v8::basic_format_string<char, to_format>]'
<source>:11:25:   required from here
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:1707:7: error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
 1707 |       formattable,
      |       ^~~~~~~~~~~
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/core.h:1707:7: note: 'formattable' evaluates to false
ASM generation compiler returned: 1
@andijcr
Copy link
Author

andijcr commented Feb 11, 2022

my bad, i missed the FMT_DEPRECATED_OSTREAM

@andijcr andijcr closed this as completed Feb 11, 2022
@jiapei-nexera
Copy link

@andijcr
Sorry, but what is your solution ?? Can you please clarify ???

@mwinterb
Copy link
Contributor

Sorry, but what is your solution ?? Can you please clarify ???

If you define FMT_DEPRECATED_OSTREAM before including any fmtlib headers, fmt will use an available ostream insertion operator. https://godbolt.org/z/54a5G1qsj

The preferred mechanism is to define a formatter for a type deriving from ostream_formatter. https://fmt.dev/dev/api.html#ostream-api

@jiapei-nexera
Copy link

@mwinterb
It looks this will bring more inconvenience ??????

For instance, I'm trying to build Open3D, which has such a file TensorMap.cpp , in which there are two occurrences of fmt::format , and they seem to be 2 different formatter from the same class.

It looks it's more friendly for the end users by using the deprecated way???

@mwinterb
Copy link
Contributor

It's not an issue of friendliness, the deprecated way caused problems, see #2357.
I don't know why TensorMap.cpp would cause issues, since it seems like the only types formatted in that file are int64_t and std::string, which are both intrinsically supported by fmt. Additionally, there doesn't seem to be any include of fmt/ostream.h in Open3D for the formatter to referenced in this issue to have been used.
The format function for the REHandle_abstract formatter needs to be const, which is a requirement that is enforced in more places than it was in earlier versions.

Additionally, this loop https://github.com/isl-org/Open3D/blob/174886a56cb8897ccb494272a2546c56527c65ce/cpp/open3d/t/geometry/TensorMap.cpp#L67-L68 is pointless since the return value of format is discarded, which is a warning in recent versions of fmt, and it seems as though Open3D compiles with warnings as errors.

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

4 participants