[seastar-dev] [PATCH v1] cmake: disable "consteval" support for libfmt 8+ when using Clang

[Pavel Solodovnikov]

Clang 13+ doesn't play nice with fmt >= 8.0.0 if
"consteval" is enabled. Turn it off.

It chokes when `fmt::format_to()` is called with a
`std::string_view` or `const char*` format string,
always trying to use consteval constructor for
`basic_format_string`.

It can be worked around by either wrapping format
strings in a `fmt::runtime()` or just disabling the
consteval support in fmtlib. The second approach
seems to be simpler.

Now the compilation passes successfully for the
default toolchain based on Fedora 35 Pre-Release.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---
CMakeLists.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a63b3b4c..7f10f792 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -981,6 +981,19 @@ if ((CMAKE_BUILD_TYPE STREQUAL "Dev") OR (CMAKE_BUILD_TYPE STREQUAL "Debug"))
SEASTAR_TYPE_ERASE_MORE)
endif ()

+# Clang 13+ doesn't play nice with fmt >= 8.0.0 if "consteval" is enabled. Turn it off.
+#
+# It chokes when `fmt::format_to()` is called with a `std::string_view` or `const char*`
+# format string, always trying to use consteval constructor for `basic_format_string`.
+#
+# It can be worked around by either wrapping format strings in a `fmt::runtime()` or
+# just disabling the consteval support in fmtlib. The second approach seems to be simpler.
+if (CMAKE_CXX_COMPILER_ID MATCHES Clang AND
+ CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 13 AND
+ fmt_VERSION VERSION_GREATER_EQUAL 8.0.0)
+ target_compile_definitions (seastar PUBLIC FMT_CONSTEVAL=)
+endif()
+
target_compile_definitions (seastar
PRIVATE ${Seastar_PRIVATE_COMPILE_DEFINITIONS})

--
2.32.0

[Avi Kivity]

On 22/09/2021 23.59, Pavel Solodovnikov wrote:
> Clang 13+ doesn't play nice with fmt >= 8.0.0 if
> "consteval" is enabled. Turn it off.
>
> It chokes when `fmt::format_to()` is called with a
> `std::string_view` or `const char*` format string,
> always trying to use consteval constructor for
> `basic_format_string`.
>
> It can be worked around by either wrapping format
> strings in a `fmt::runtime()` or just disabling the
> consteval support in fmtlib. The second approach
> seems to be simpler.
>
> Now the compilation passes successfully for the
> default toolchain based on Fedora 35 Pre-Release.

Please report an issue at fmtlib. We may get a better workaround. The
workaround below is okay, but it's somewhat open-ended - we won't get
compile-time format strings if we forget to adjust the workaround for
newer compilers/fmts.

[Pavel Solodovnikov]

On Thu, Sep 23, 2021 at 11:48 AM Avi Kivity <a...@scylladb.com> wrote:
>
>
> On 22/09/2021 23.59, Pavel Solodovnikov wrote:
> > Clang 13+ doesn't play nice with fmt >= 8.0.0 if
> > "consteval" is enabled. Turn it off.
> >
> > It chokes when `fmt::format_to()` is called with a
> > `std::string_view` or `const char*` format string,
> > always trying to use consteval constructor for
> > `basic_format_string`.
> >
> > It can be worked around by either wrapping format
> > strings in a `fmt::runtime()` or just disabling the
> > consteval support in fmtlib. The second approach
> > seems to be simpler.
> >
> > Now the compilation passes successfully for the
> > default toolchain based on Fedora 35 Pre-Release.
>
>
> Please report an issue at fmtlib. We may get a better workaround. The
> workaround below is okay, but it's somewhat open-ended - we won't get
> compile-time format strings if we forget to adjust the workaround for
> newer compilers/fmts.

Looks like it's not clang-specific, gcc also does fail to compile. I'll take
more time to look closely at how to best solve this.

If applicable, I'll file an issue with libfmt and also post a link here.

[Pavel Solodovnikov]

On Thu, Sep 23, 2021 at 1:33 PM Pavel Solodovnikov
<pa.solo...@scylladb.com> wrote:
>
> On Thu, Sep 23, 2021 at 11:48 AM Avi Kivity <a...@scylladb.com> wrote:
> >
> >
> > On 22/09/2021 23.59, Pavel Solodovnikov wrote:
> > > Clang 13+ doesn't play nice with fmt >= 8.0.0 if
> > > "consteval" is enabled. Turn it off.
> > >
> > > It chokes when `fmt::format_to()` is called with a
> > > `std::string_view` or `const char*` format string,
> > > always trying to use consteval constructor for
> > > `basic_format_string`.
> > >
> > > It can be worked around by either wrapping format
> > > strings in a `fmt::runtime()` or just disabling the
> > > consteval support in fmtlib. The second approach
> > > seems to be simpler.
> > >
> > > Now the compilation passes successfully for the
> > > default toolchain based on Fedora 35 Pre-Release.
> >
> >
> > Please report an issue at fmtlib. We may get a better workaround. The
> > workaround below is okay, but it's somewhat open-ended - we won't get
> > compile-time format strings if we forget to adjust the workaround for
> > newer compilers/fmts.
> Looks like it's not clang-specific, gcc also does fail to compile. I'll take
> more time to look closely at how to best solve this.
>
> If applicable, I'll file an issue with libfmt and also post a link here.

Turns out this is expected behavior, please see:
https://github.com/fmtlib/fmt/issues/2421

Format strings are always assumed to be constant-evaluated, otherwise
an explicit call to fmt::runtime is needed.
The logging functions are not consteval/constexpr, hence we cannot use
consteval format strings.

I've prepared a patch to conditionally switch to the new API. Will
send in a minute.

[Pavel Solodovnikov]

Sent, topic name is: [seastar-dev] [PATCH v1] build: adjust
compilation for libfmt 8+

On Fri, Oct 1, 2021 at 3:14 PM Pavel Solodovnikov