Skip to content

Conversation

@mikoff
Copy link
Contributor

@mikoff mikoff commented Jul 7, 2025

I have created a simple fix to enable symforce compilation for c++20 by wrapping the runtime string into fmt::runtime.

There is one downside though: we lose compile-time format-string checking for some assert-message payloads.

@mikoff mikoff changed the title fix fmt::format for c++20 C++20 build fix: wrap fmt argument with fmt::runtime Jul 7, 2025
@mikoff mikoff mentioned this pull request Jul 7, 2025
Copy link
Member

@aaron-skydio aaron-skydio left a comment

Choose a reason for hiding this comment

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

Thanks! I'll go ahead and test. I think this is independent of supporting fmtlib 9+ which is also a thing I have in progress that I haven't merged yet...

Comment on lines 25 to 31
template <typename... T>
inline std::string FormatFailure(const char* error, const char* func, const char* file, int line,
const char* fmt, T&&... args) {
return fmt::format("SYM_ASSERT: {}\n --> {}\n --> {}:{}\n{}\n", error, func, file, line,
fmt::format(fmt, std::forward<T>(args)...));
return fmt::format("SYM_ASSERT: {}\n --> {}\n --> {}:{}\n{}\n",
error, func, file, line,
fmt::format(fmt::runtime(fmt), std::forward<T>(args)...));
}
Copy link
Member

Choose a reason for hiding this comment

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

I think instead of this, we can write:

template <typename... T>
inline std::string FormatFailure(const char* error, const char* func, const char* file, int line,
                                 fmt::format_string<T...> fmt, T&&... args) {
  return fmt::format("SYM_ASSERT: {}\n    --> {}\n    --> {}:{}\n{}\n", error, func, file, line,
                     fmt::format(fmt, std::forward<T>(args)...));
}

and preserve the compile-time checking

Copy link
Contributor Author

@mikoff mikoff Jul 8, 2025

Choose a reason for hiding this comment

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

@aaron-skydio thanks for looking into it - I changed the FormatFailure definition according to your suggestion, everything compiles and runs well.

Clang-styled code formatting was also fixed. So there should be no other issues.

Could you approve it again?

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 this pull request may close these issues.

2 participants