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

build fails due to fmt::format being consteval on C++20 #492

Closed
pgbarletta opened this issue Feb 26, 2024 · 5 comments
Closed

build fails due to fmt::format being consteval on C++20 #492

pgbarletta opened this issue Feb 26, 2024 · 5 comments

Comments

@pgbarletta
Copy link
Member

Hi!

I have a C++20 project and I was getting these errors when compiling against chemfiles with C++20:

In file included from /home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/src/Connectivity.cpp:11:
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp: In instantiation of ‘chemfiles::Error chemfiles::error(const char*, Args&& ...) [with Args = {}]’:
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/src/Connectivity.cpp:18:20:   required from here
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:22:29:   in ‘constexpr’ expansion of ‘fmt::v10::basic_format_string<char>(format)’
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:22:29: error: ‘format’ is not a constant expression
   22 |     return Error(fmt::format(format, std::forward<Args>(arguments)...));
      |                  ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 49%] Building C object _deps/chemfiles-build/external/lzma/CMakeFiles/chemfiles_lzma.dir/src/liblzma/common/filter_buffer_decoder.c.o
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp: In instantiation of ‘chemfiles::OutOfBounds chemfiles::out_of_bounds(const char*, Args&& ...) [with Args = {long unsigned int&}]’:
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/src/Connectivity.cpp:27:28:   required from here
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:52:35:   in ‘constexpr’ expansion of ‘fmt::v10::basic_format_string<char, long unsigned int&>(format)’
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:52:35: error: ‘format’ is not a constant expression
   52 |     return OutOfBounds(fmt::format(format, std::forward<Args>(arguments)...));
      |                        ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp: In instantiation of ‘chemfiles::Error chemfiles::error(const char*, Args&& ...) [with Args = {long unsigned int&, long unsigned int&}]’:
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/src/Connectivity.cpp:252:16:   required from here
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:22:29:   in ‘constexpr’ expansion of ‘fmt::v10::basic_format_string<char, long unsigned int&, long unsigned int&>(format)’
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-src/include/chemfiles/error_fmt.hpp:22:29: error: ‘format’ is not a constant expression
   22 |     return Error(fmt::format(format, std::forward<Args>(arguments)...));

switching the standard to C++17 fixed the issue.

To confirm this, I cloned chemfiles and built from source using different configuration steps:

# This one worked
cmake -S. -Bbuild
# This one failed with the same issues shown above
cmake -S. -Bbuild -DCMAKE_CXX_STANDARD=20

Apparently fmt::format is consteval since C++20 and expects a constant expression in the format string.

Solutions I can think of

  • A quick fix consists on replacing all occurrences of const char * in error_fmt.hpp and warnings.hpp with fmt::format_string<Args...> which does work since the library is indeed sending strings known at compile time each time it uses these error and warning functions. This one does work, but it's a break for users that are using chemfiles error functions and sending them runtime strings.
  • Same as above, but instead of replacing the functions, just adding new overloads so the 2 options coexist, const char * and fmt::format_string<Args...>.
  • Instead of overloading on the type of the format string, overload functions but put the format string as a non-type template parameter. Then replace the usages of these error functions throughout the libraries.
  • Replace fmt::format with fmt::vformat, that does work with runtime strings. The drawback is that you loose compile-time check of the format string.

Some extra references:

@Luthaf
Copy link
Member

Luthaf commented Feb 26, 2024

Hmm, this is strange. We are asking cmake to build with the C++17 standard:

target_compile_features(chemfiles_objects PRIVATE cxx_std_17 c_std_99)

The error above occurs during the compilation of chemfiles_objects, right?


Might be that CMAKE_CXX_STANDARD=20 overrides the cxx_std_17 target property. What happens if you set the cxx_std_20 target property only for your own targets instead of the full project?

@pgbarletta
Copy link
Member Author

Yeap! This fixes it:

target_compile_features(myproject PRIVATE cxx_std_20)
set_target_properties(myproject PROPERTIES CXX_EXTENSIONS OFF)

Thanks!

@pgbarletta
Copy link
Member Author

Sorry, but now I'm getting linking errors due to fmt and chemfiles conflicting.

Something like this:


#include <chemfiles.hpp>
#include <fmt/core.h>

int main() {
    chemfiles::Trajectory trj("a.pdb");

    fmt::print("\n");
}

Will trigger linking errors from format-inl.h:

/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::is_constant_evaluated(bool)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:30: multiple definition of `fmt::v10::detail::assert_fail(char const*, int, char const*)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:30: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::is_printable(unsigned int)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1511: multiple definition of `fmt::v10::detail::is_printable(unsigned int)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1739: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `int fmt::v10::detail::count_digits_fallback<unsigned __int128>(unsigned __int128)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:44: multiple definition of `fmt::v10::detail::format_error_code(fmt::v10::detail::buffer<char>&, int, fmt::v10::basic_string_view<char>)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:40: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `void fmt::v10::detail::parse_format_string<false, char, fmt::v10::detail::vformat_to<char>(fmt::v10::detail::buffer<char>&, fmt::v10::basic_string_view<char>, fmt::v10::detail::vformat_args<char>::type, fmt::v10::detail::locale_ref)::format_handler>(fmt::v10::basic_string_view<char>, fmt::v10::detail::vformat_to<char>(fmt::v10::detail::buffer<char>&, fmt::v10::basic_string_view<char>, fmt::v10::detail::vformat_args<char>::type, fmt::v10::detail::locale_ref)::format_handler&&)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:67: multiple definition of `fmt::v10::detail::report_error(void (*)(fmt::v10::detail::buffer<char>&, int, char const*), int, char const*)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:63: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::dragonbox::get_cached_power(int)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1115: multiple definition of `fmt::v10::detail::dragonbox::get_cached_power(int)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1126: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::utf8_to_utf16::utf8_to_utf16(fmt::v10::basic_string_view<char>)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1387: multiple definition of `fmt::v10::detail::utf8_to_utf16::utf8_to_utf16(fmt::v10::basic_string_view<char>)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1398: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::utf8_to_utf16::utf8_to_utf16(fmt::v10::basic_string_view<char>)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1387: multiple definition of `fmt::v10::detail::utf8_to_utf16::utf8_to_utf16(fmt::v10::basic_string_view<char>)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1398: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::format_system_error(fmt::v10::detail::buffer<char>&, int, char const*)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1403: multiple definition of `fmt::v10::format_system_error(fmt::v10::detail::buffer<char>&, int, char const*)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1414: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::report_system_error(int, char const*)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1414: multiple definition of `fmt::v10::report_system_error(int, char const*)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1425: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::write_console(int, fmt::v10::basic_string_view<char>)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1428: multiple definition of `fmt::v10::detail::write_console(int, fmt::v10::basic_string_view<char>)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1643: first defined here
/usr/bin/ld: _deps/chemfiles-build/libchemfiles.a(format.cc.o): in function `fmt::v10::detail::print(_IO_FILE*, fmt::v10::basic_string_view<char>)':
/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/chemfiles-build/external/fmt/include/fmt/format-inl.h:1450: multiple definition of `fmt::v10::detail::print(_IO_FILE*, fmt::v10::basic_string_view<char>)'; _deps/fmt-build/libfmtd.a(format.cc.o):/home/pbarletta/labo/23/test_stdexec/bbuild/_deps/fmt-src/include/fmt/format-inl.h:1667: first defined here
collect2: error: ld returned 1 exit status

Apparently this file is only used when fmtlib is used in header-only format:

#ifdef FMT_HEADER_ONLY
#  define FMT_FUNC inline
#  include "format-inl.h"
#else
#  define FMT_FUNC
#endif

But I'm pretty sure I'm not doing that and neither does chemfiles. Maybe it's the way I'm getting both libraries, I'm using CPM to get the packages directly from github.

Have you ever run into anything like this?

@Luthaf
Copy link
Member

Luthaf commented Feb 28, 2024

So I was trying to prevent this kind of issue by making all the symbols hidden: https://github.com/chemfiles/fmt/blob/6fc2a95218a50f69d8d54629645bf954e4ed89bb/0-chemfiles/CMakeLists.txt#L15-L16

But there might be something wrong here. Does it work if you build chemfiles as a shared library? I'm not sure how static libraries handle hidden symbols.

Another workaround might be to only build format.cc once, inside chemfiles and not inside your project. This is not a good solution long term but might fix your build while we figure out the best solution.


I'm using CPM to get the packages directly from github.

What's CPM?

@pgbarletta
Copy link
Member Author

Generating a shared library actually fixes it, thanks again.

CPM is like FetchContent() on steroids, so I was getting chemfiles directly from github. The thing is that I couldn't set BUILD_SHARED_LIBS from my CMakeLists.txt, so every time I built my project, chemfiles would build as a static library.

Now I'm just including the chemfiles dir along my project. with BUILD_SHARED_LIBS=ON hardcoded.

Thank you again for your help.

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

No branches or pull requests

2 participants