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

Compile issues with libfmt >=8.1.0 #2935

Closed
blattms opened this issue Jan 21, 2022 · 5 comments · Fixed by #2938
Closed

Compile issues with libfmt >=8.1.0 #2935

blattms opened this issue Jan 21, 2022 · 5 comments · Fixed by #2938

Comments

@blattms
Copy link
Member

blattms commented Jan 21, 2022

Seems like newer versions are less forgiving when formatting enum classes.


/usr/bin/c++ -DBOOST_ALL_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DEMBEDDED_PYTHON -DFMT_LOCALE -DFMT_SHARED -DHAVE_CONFIG_H=1 -Dopmcommon_EXPORTS -I/build/opm-common-2021.10/obj-x86_64-linux-gnu -I/build/opm-common-2021.10/obj-x86_64-linux-gnu/include -I/build/opm-common-2021.10 -I/usr/include/cjson -isystem /usr/include/python3.9 -g -O2 -ffile-prefix-map=/build/opm-common-2021.10=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -pipe -fopenmp -pthread -fPIC -fopenmp -std=c++17 -MD -MT CMakeFiles/opmcommon.dir/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQState.cpp.o -MF CMakeFiles/opmcommon.dir/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQState.cpp.o.d -o CMakeFiles/opmcommon.dir/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQState.cpp.o -c /build/opm-common-2021.10/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQState.cpp
In file included from /usr/include/fmt/format.h:48,
                 from /build/opm-common-2021.10/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQSet.cpp:22:
/usr/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 = Opm::UDQVarType&; typename std::enable_if<IS_PACKED, int>::type <anonymous> = 0]':
/usr/include/fmt/core.h:1855:77:   required from 'constexpr fmt::v8::format_arg_store<Context, Args>::format_arg_store(T&& ...) [with T = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int&, Opm::UDQVarType&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int&, Opm::UDQVarType&}; Context = fmt::v8::basic_format_context<fmt::v8::appender, char>; Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, long unsigned int, Opm::UDQVarType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, long unsigned int, Opm::UDQVarType}]'
/usr/include/fmt/core.h:1872: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 = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int&, Opm::UDQVarType&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int&, Opm::UDQVarType&}]'
/usr/include/fmt/core.h:3119:44:   required from 'std::string fmt::v8::format(fmt::v8::format_string<T ...>, T&& ...) [with T = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int, Opm::UDQVarType, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int, Opm::UDQVarType}; std::string = std::__cxx11::basic_string<char>; fmt::v8::format_string<T ...> = fmt::v8::basic_format_string<char, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int, Opm::UDQVarType, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, long unsigned int, Opm::UDQVarType>]'
/build/opm-common-2021.10/src/opm/parser/eclipse/EclipseState/Schedule/UDQ/UDQSet.cpp:523:27:   required from here
/usr/include/fmt/core.h:1728: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
 1728 |       formattable,
      |       ^~~~~~~~~~~
/usr/include/fmt/core.h:1728:7: note: 'formattable' evaluates to false

The problem is that enum classes are not anymore cast internally to int (or similar). See comment on issue 391 of fmt, issue /fmtlib/fmt#1841, and commit fd62fba985.
We either need to provide a formatter, cast to int when formatting, or provide operator<< for all enum classes used with format. To me the latter seems most intriguing.

@blattms blattms changed the title Compile issues with libfmt 8.1.1 Compile issues with libfmt >=8.1.0 Jan 21, 2022
@blattms
Copy link
Member Author

blattms commented Jan 21, 2022

87 enum classes in opm-common. Wondering how many are used in fmt::format. Guess I'll need to try.

@atgeirr
Copy link
Member

atgeirr commented Jan 25, 2022

We have quite a few "enum to string" facilities in opm-common, is this better done with operator<< instead? Is the opposite operator possible to define as well, for "string to enum" conversion?

@bska
Copy link
Member

bska commented Jan 25, 2022

We have quite a few "enum to string" facilities in opm-common, is this better done with operator<< instead?

I don't really see how we can define an operator<<() overload that converts an enum into a string with no other context. Those output operators typically take a std::basic_ostream<>& as their first argument and are specifically intended to write values into an output stream.

@atgeirr
Copy link
Member

atgeirr commented Jan 25, 2022

I don't really see how we can define an operator<<() overload that converts an enum into a string with no other context. Those output operators typically take a std::basic_ostream<>& as their first argument and are specifically intended to write values into an output stream.

I was implicitly assuming the facility was mostly used for logging, when we often have a stream, but that may not be the case? The distinction between doing stream insertion and string conversion is always annoying me...

@bska
Copy link
Member

bska commented Jan 25, 2022

I don't really see how we can define an operator<<() overload that converts an enum into a string with no other context. Those output operators typically take a std::basic_ostream<>& as their first argument and are specifically intended to write values into an output stream.

I was implicitly assuming the facility was mostly used for logging, when we often have a stream, but that may not be the case? The distinction between doing stream insertion and string conversion is always annoying me...

I think operator<<() would work absolutely fine in the context of logging, but we also have a great many locations that use

throw some_exception_type {
    "Unsupported option value '" + stringify(enum_value) + '\''
};

and that's more difficult to handle with operator<<(). Of course if we were more disciplined in our error handling...

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

Successfully merging a pull request may close this issue.

3 participants