dhaglin opened a new issue, #700:
URL: https://github.com/apache/logging-log4cxx/issues/700
# AsyncBuffer fails to compile with {fmt} 11+ (format_string →
basic_string_view binds as const lvalue)
## Summary
The `LOG4CXX_*_FMT_ASYNC` macros do not compile against **{fmt} 11 or 12**.
`AsyncBuffer::setMessage` passes the `fmt::format_string` straight to
`initializeForFmt`, which takes its format argument by rvalue reference
(`StringViewType&&`). As of {fmt} 11, the implicit
`fmt::basic_format_string → fmt::basic_string_view` conversion yields a
**const
lvalue reference** rather than a prvalue, so it no longer binds to the
rvalue-reference parameter and the build fails.
This is still present on `master` (the `std::move(fmt_str)` calls are
unchanged).
It is independent of #563, which addressed a GCC 12 / C++20-concepts failure
in
the same class but did not touch these lines.
## Affected code
`src/main/include/log4cxx/helpers/asyncbuffer.h` — both `setMessage`
overloads:
```cpp
// narrow (around line 207 on master)
void setMessage(fmt::format_string<Args...> fmt_str, Args&&... args)
{
auto store = FmtArgStore();
( store.push_back(std::forward<Args>(args)), ...);
initializeForFmt(std::move(fmt_str), std::move(store)); // <-- fails
on fmt 11+
}
// wide (around line 218 on master)
void setMessage(fmt::wformat_string<Args...> fmt_str, Args&&... args)
{
auto store = WideFmtArgStore();
( store.push_back(std::forward<Args>(args)), ...);
initializeForFmt(std::move(fmt_str), std::move(store)); // <-- fails
on fmt 11+
}
```
These call the private declarations:
```cpp
void initializeForFmt(StringViewType&& format_string, FmtArgStore&& args);
void initializeForFmt(WideStringViewType&& format_string, WideFmtArgStore&&
args);
```
## Environment
- log4cxx: 1.7.0 and current `master`
- {fmt}: 11.x and 12.x (12.x is what we build against)
- C++20
- Reproduced with both Clang and GCC; not compiler-specific.
## Reproduction
Build with `LOG4CXX_*_ASYNC` enabled against {fmt} 11+ and use any async fmt
macro, e.g.:
```cpp
LOG4CXX_INFO_FMT_ASYNC(logger, "value {}", 42);
```
The bundled test `src/test/cpp/asyncappendertestcase.cpp` also fails to
compile
under {fmt} 11+ for the same reason.
## Root cause
`fmt::basic_format_string`'s conversion to `basic_string_view` changed in
{fmt} 11 to return a const lvalue reference. An rvalue-reference parameter
(`StringViewType&&`) cannot bind to a const lvalue, so overload resolution
for
`initializeForFmt` fails.
## Proposed fix
Call `.get()` on the format string, which returns the `basic_string_view` by
value (a prvalue) and binds correctly on {fmt} 11 and 12. We verified this
against {fmt} 12.x. One thing to confirm on your side: that `.get()` is
present
on the lowest {fmt} version log4cxx still supports, so this doesn't regress
older {fmt}.
```diff
auto store = FmtArgStore();
( store.push_back(std::forward<Args>(args)), ...);
- initializeForFmt(std::move(fmt_str), std::move(store));
+ initializeForFmt(fmt_str.get(), std::move(store));
```
```diff
auto store = WideFmtArgStore();
( store.push_back(std::forward<Args>(args)), ...);
- initializeForFmt(std::move(fmt_str), std::move(store));
+ initializeForFmt(fmt_str.get(), std::move(store));
```
Happy to open a PR with this change if it looks right.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
