Skip to content

Commit

Permalink
lib/fmt: drop use of FMT_STRING
Browse files Browse the repository at this point in the history
when compiling with libfmt-11.1.0 and newer the following compile errors occur:

In file included from ../src/decoder/DecoderPrint.cxx:23:
../src/client/Response.hxx: In instantiation of 'bool Response::Fmt(const S&, Args&& ...) [with S = decoder_plugin_print(Response&, const DecoderPlugin&)::<lambda()>::FMT_COMPILE_STRING; Args = {const char* const&}]':
../src/decoder/DecoderPrint.cxx:38:7:   required from here
   38 |         r.Fmt(FMT_STRING("plugin: {}\n"), plugin.name);
      |         ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/client/Response.hxx:86:28: error: cannot convert 'const decoder_plugin_print(Response&, const DecoderPlugin&)::<lambda()>::FMT_COMPILE_STRING' to 'fmt::v11::string_view' {aka 'fmt::v11::basic_string_view<char>'}
   86 |                 return VFmt(format_str,
      |                        ~~~~^~~~~~~~~~~~
   87 |                             fmt::make_format_args(args...));
      |                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/client/Response.hxx:81:36: note: initializing argument 1 of 'bool Response::VFmt(fmt::v11::string_view, fmt::v11::format_args)'
   81 |         bool VFmt(fmt::string_view format_str, fmt::format_args args) noexcept;
      |                   ~~~~~~~~~~~~~~~~~^~~~~~~~~~
../src/client/Response.hxx: In instantiation of 'bool Response::Fmt(const S&, Args&& ...) [with S = decoder_plugin_print(Response&, const DecoderPlugin&)::<lambda()>::FMT_COMPILE_STRING; Args = {const char* const&}]':

The error is due to the use of FMT_STRING. The libfmt team provided the following:

    The correct way of using FMT_STRING is to wrap a format string when passing to a
    function with compile-time checks (i.e. that takes format_string) as documented
    in https://fmt.dev/11.1/api/#legacy-compile-time-checks.

    Noting that FMT_STRING is a legacy API and has been superseded by consteval-based
    API starting from version 8: https://github.com/fmtlib/fmt/releases/tag/8.0.0. It
    looks like MPD is trying to emulate {fmt}'s old way of implementing compile-time
    checks which was never properly documented because it was basically a hack. So the
    correct fix is to switch to format_string and, possibly, remove usage of FMT_STRING.

    The old way of doing compile-time checks (fmt::make_args_checked) was documented
    in https://fmt.dev/7.1/api.html#argument-lists but it looks like MPD is not using
    that API so the problematic uses of FMT_STRING have no effect and can just be removed.

The FMT_STRING has been removed in this change based on the fmt-7.1 API and now MPD is
successfully compile against the current libfmt-11.1.0 which highlighted the issue that
had been present in the codebase as it is now triggering the error, is legacy and was
not using the API for which FMT_STRING was aligned with.
  • Loading branch information
heitbaum committed Jan 13, 2025
1 parent b5bd294 commit 6efba9f
Show file tree
Hide file tree
Showing 25 changed files with 110 additions and 110 deletions.
20 changes: 10 additions & 10 deletions src/SongPrint.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ song_print_uri(Response &r, const char *uri, bool base) noexcept
uri = allocated.c_str();
}

r.Fmt(FMT_STRING(SONG_FILE "{}\n"), uri);
r.Fmt(SONG_FILE "{}\n", uri);
}

void
song_print_uri(Response &r, const LightSong &song, bool base) noexcept
{
if (!base && song.directory != nullptr)
r.Fmt(FMT_STRING(SONG_FILE "{}/{}\n"),
r.Fmt(SONG_FILE "{}/{}\n",
song.directory, song.uri);
else
song_print_uri(r, song.uri, base);
Expand All @@ -72,13 +72,13 @@ PrintRange(Response &r, SongTime start_time, SongTime end_time) noexcept
const unsigned end_ms = end_time.ToMS();

if (end_ms > 0)
r.Fmt(FMT_STRING("Range: {}.{:03}-{}.{:03}\n"),
r.Fmt("Range: {}.{:03}-{}.{:03}\n",
start_ms / 1000,
start_ms % 1000,
end_ms / 1000,
end_ms % 1000);
else if (start_ms > 0)
r.Fmt(FMT_STRING("Range: {}.{:03}-\n"),
r.Fmt("Range: {}.{:03}-\n",
start_ms / 1000,
start_ms % 1000);
}
Expand All @@ -94,14 +94,14 @@ song_print_info(Response &r, const LightSong &song, bool base) noexcept
time_print(r, "Last-Modified", song.mtime);

if (song.audio_format.IsDefined())
r.Fmt(FMT_STRING("Format: {}\n"), song.audio_format);
r.Fmt("Format: {}\n", song.audio_format);

tag_print_values(r, song.tag);

const auto duration = song.GetDuration();
if (!duration.IsNegative())
r.Fmt(FMT_STRING("Time: {}\n"
"duration: {:1.3f}\n"),
r.Fmt("Time: {}\n"
"duration: {:1.3f}\n",
duration.RoundS(),
duration.ToDoubleS());
}
Expand All @@ -117,14 +117,14 @@ song_print_info(Response &r, const DetachedSong &song, bool base) noexcept
time_print(r, "Last-Modified", song.GetLastModified());

if (const auto &f = song.GetAudioFormat(); f.IsDefined())
r.Fmt(FMT_STRING("Format: {}\n"), f);
r.Fmt("Format: {}\n", f);

tag_print_values(r, song.GetTag());

const auto duration = song.GetDuration();
if (!duration.IsNegative())
r.Fmt(FMT_STRING("Time: {}\n"
"duration: {:1.3f}\n"),
r.Fmt("Time: {}\n"
"duration: {:1.3f}\n",
duration.RoundS(),
duration.ToDoubleS());
}
14 changes: 7 additions & 7 deletions src/Stats.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,18 @@ db_stats_print(Response &r, const Database &db)
unsigned total_duration_s =
std::chrono::duration_cast<std::chrono::seconds>(stats.total_duration).count();

r.Fmt(FMT_STRING("artists: {}\n"
"albums: {}\n"
"songs: {}\n"
"db_playtime: {}\n"),
r.Fmt("artists: {}\n"
"albums: {}\n"
"songs: {}\n"
"db_playtime: {}\n",
stats.artist_count,
stats.album_count,
stats.song_count,
total_duration_s);

const auto update_stamp = db.GetUpdateStamp();
if (!IsNegative(update_stamp))
r.Fmt(FMT_STRING("db_update: {}\n"),
r.Fmt("db_update: {}\n",
std::chrono::system_clock::to_time_t(update_stamp));
}

Expand All @@ -125,8 +125,8 @@ stats_print(Response &r, const Partition &partition)
const auto uptime = std::chrono::steady_clock::now() - start_time;
#endif

r.Fmt(FMT_STRING("uptime: {}\n"
"playtime: {}\n"),
r.Fmt("uptime: {}\n"
"playtime: {}\n",
std::chrono::duration_cast<std::chrono::seconds>(uptime).count(),
lround(partition.pc.GetTotalPlayTime().count()));

Expand Down
10 changes: 5 additions & 5 deletions src/TagPrint.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,20 @@ tag_print_types(Response &r) noexcept
const auto tag_mask = global_tag_mask & r.GetTagMask();
for (unsigned i = 0; i < TAG_NUM_OF_ITEM_TYPES; i++)
if (tag_mask.Test(TagType(i)))
r.Fmt(FMT_STRING("tagtype: {}\n"), tag_item_names[i]);
r.Fmt("tagtype: {}\n", tag_item_names[i]);
}

void
tag_print(Response &r, TagType type, StringView _value) noexcept
{
const std::string_view value{_value};
r.Fmt(FMT_STRING("{}: {}\n"), tag_item_names[type], value);
r.Fmt("{}: {}\n", tag_item_names[type], value);
}

void
tag_print(Response &r, TagType type, const char *value) noexcept
{
r.Fmt(FMT_STRING("{}: {}\n"), tag_item_names[type], value);
r.Fmt("{}: {}\n", tag_item_names[type], value);
}

void
Expand All @@ -60,8 +60,8 @@ void
tag_print(Response &r, const Tag &tag) noexcept
{
if (!tag.duration.IsNegative())
r.Fmt(FMT_STRING("Time: {}\n"
"duration: {:1.3f}\n"),
r.Fmt("Time: {}\n"
"duration: {:1.3f}\n",
tag.duration.RoundS(),
tag.duration.ToDoubleS());

Expand Down
2 changes: 1 addition & 1 deletion src/TimePrint.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ time_print(Response &r, const char *name,
return;
}

r.Fmt(FMT_STRING("{}: {}\n"), name, s.c_str());
r.Fmt("{}: {}\n", name, s.c_str());
}
2 changes: 1 addition & 1 deletion src/client/Idle.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ WriteIdleResponse(Response &r, unsigned flags) noexcept
const char *const*idle_names = idle_get_names();
for (unsigned i = 0; idle_names[i]; ++i) {
if (flags & (1 << i))
r.Fmt(FMT_STRING("changed: {}\n"), idle_names[i]);
r.Fmt("changed: {}\n", idle_names[i]);
}

r.Write("OK\n");
Expand Down
4 changes: 2 additions & 2 deletions src/client/Response.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Response::WriteBinary(ConstBuffer<void> payload) noexcept
void
Response::Error(enum ack code, const char *msg) noexcept
{
Fmt(FMT_STRING("ACK [{}@{}] {{{}}} "),
Fmt("ACK [{}@{}] {{{}}} ",
(int)code, list_index, command);

Write(msg);
Expand All @@ -77,7 +77,7 @@ void
Response::VFmtError(enum ack code,
fmt::string_view format_str, fmt::format_args args) noexcept
{
Fmt(FMT_STRING("ACK [{}@{}] {{{}}} "),
Fmt("ACK [{}@{}] {{{}}} ",
(int)code, list_index, command);

VFmt(format_str, args);
Expand Down
14 changes: 7 additions & 7 deletions src/command/AllCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ PrintAvailableCommands(Response &r, const Partition &partition,

if (cmd->permission == (permission & cmd->permission) &&
command_available(partition, cmd))
r.Fmt(FMT_STRING("command: {}\n"), cmd->cmd);
r.Fmt("command: {}\n", cmd->cmd);
}

return CommandResult::OK;
Expand All @@ -268,7 +268,7 @@ PrintUnavailableCommands(Response &r, unsigned permission) noexcept
const struct command *cmd = &i;

if (cmd->permission != (permission & cmd->permission))
r.Fmt(FMT_STRING("command: {}\n"), cmd->cmd);
r.Fmt("command: {}\n", cmd->cmd);
}

return CommandResult::OK;
Expand Down Expand Up @@ -326,7 +326,7 @@ command_check_request(const struct command *cmd, Response &r,
{
if (cmd->permission != (permission & cmd->permission)) {
r.FmtError(ACK_ERROR_PERMISSION,
FMT_STRING("you don't have permission for \"{}\""),
"you don't have permission for \"{}\"",
cmd->cmd);
return false;
}
Expand All @@ -339,17 +339,17 @@ command_check_request(const struct command *cmd, Response &r,

if (min == max && unsigned(max) != args.size) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("wrong number of arguments for \"{}\""),
"wrong number of arguments for \"{}\"",
cmd->cmd);
return false;
} else if (args.size < unsigned(min)) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("too few arguments for \"{}\""),
"too few arguments for \"{}\"",
cmd->cmd);
return false;
} else if (max >= 0 && args.size > unsigned(max)) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("too many arguments for \"{}\""),
"too many arguments for \"{}\"",
cmd->cmd);
return false;
} else
Expand All @@ -363,7 +363,7 @@ command_checked_lookup(Response &r, unsigned permission,
const struct command *cmd = command_lookup(cmd_name);
if (cmd == nullptr) {
r.FmtError(ACK_ERROR_UNKNOWN,
FMT_STRING("unknown command \"{}\""), cmd_name);
"unknown command \"{}\"", cmd_name);
return nullptr;
}

Expand Down
8 changes: 4 additions & 4 deletions src/command/DatabaseCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ handle_count(Client &client, Request args, Response &r)
group = tag_name_parse_i(s);
if (group == TAG_NUM_OF_ITEM_TYPES) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("Unknown tag type: {}"), s);
"Unknown tag type: {}", s);
return CommandResult::ERROR;
}

Expand Down Expand Up @@ -312,7 +312,7 @@ handle_list(Client &client, Request args, Response &r)
const auto tagType = tag_name_parse_i(tag_name);
if (tagType == TAG_NUM_OF_ITEM_TYPES) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("Unknown tag type: {}"), tag_name);
"Unknown tag type: {}", tag_name);
return CommandResult::ERROR;
}

Expand All @@ -326,7 +326,7 @@ handle_list(Client &client, Request args, Response &r)
/* for compatibility with < 0.12.0 */
if (tagType != TAG_ALBUM) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("should be \"{}\" for 3 arguments"),
"should be \"{}\" for 3 arguments",
tag_item_names[TAG_ALBUM]);
return CommandResult::ERROR;
}
Expand All @@ -341,7 +341,7 @@ handle_list(Client &client, Request args, Response &r)
const auto group = tag_name_parse_i(s);
if (group == TAG_NUM_OF_ITEM_TYPES) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("Unknown tag type: {}"), s);
"Unknown tag type: {}", s);
return CommandResult::ERROR;
}

Expand Down
14 changes: 7 additions & 7 deletions src/command/FileCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ handle_listfiles_local(Response &r, Path path_fs)
continue;

if (fi.IsRegular())
r.Fmt(FMT_STRING("file: {}\n"
"size: {}\n"),
r.Fmt("file: {}\n"
"size: {}\n",
name_utf8,
fi.GetSize());
else if (fi.IsDirectory())
r.Fmt(FMT_STRING("directory: {}\n"), name_utf8);
r.Fmt("directory: {}\n", name_utf8);
else
continue;

Expand Down Expand Up @@ -129,7 +129,7 @@ class PrintCommentHandler final : public NullTagHandler {
void OnPair(StringView _key, StringView _value) noexcept override {
const std::string_view key{_key}, value{_value};
if (IsValidName(key) && IsValidValue(value))
response.Fmt(FMT_STRING("{}: {}\n"), key, value);
response.Fmt("{}: {}\n", key, value);
}
};

Expand Down Expand Up @@ -218,7 +218,7 @@ read_stream_art(Response &r, const std::string_view art_directory,
read_size = is->Read(lock, buffer.get(), buffer_size);
}

r.Fmt(FMT_STRING("size: {}\n"), art_file_size);
r.Fmt("size: {}\n", art_file_size);

r.WriteBinary({buffer.get(), read_size});

Expand Down Expand Up @@ -348,10 +348,10 @@ class PrintPictureHandler final : public NullTagHandler {
return;
}

response.Fmt(FMT_STRING("size: {}\n"), buffer.size);
response.Fmt("size: {}\n", buffer.size);

if (mime_type != nullptr)
response.Fmt(FMT_STRING("type: {}\n"), mime_type);
response.Fmt("type: {}\n", mime_type);

buffer.size -= offset;

Expand Down
4 changes: 2 additions & 2 deletions src/command/MessageCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ handle_channels(Client &client, [[maybe_unused]] Request args, Response &r)
}

for (const auto &channel : channels)
r.Fmt(FMT_STRING("channel: {}\n"), channel);
r.Fmt("channel: {}\n", channel);

return CommandResult::OK;
}
Expand All @@ -99,7 +99,7 @@ handle_read_messages(Client &client,
assert(args.empty());

client.ConsumeMessages([&r](const auto &msg){
r.Fmt(FMT_STRING("channel: {}\nmessage: {}\n"),
r.Fmt("channel: {}\nmessage: {}\n",
msg.GetChannel(), msg.GetMessage());
});

Expand Down
12 changes: 6 additions & 6 deletions src/command/OtherCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static void
print_spl_list(Response &r, const PlaylistVector &list)
{
for (const auto &i : list) {
r.Fmt(FMT_STRING("playlist: {}\n"), i.name);
r.Fmt("playlist: {}\n", i.name);

if (!IsNegative(i.mtime))
time_print(r, "Last-Modified", i.mtime);
Expand Down Expand Up @@ -249,7 +249,7 @@ handle_update(Response &r, UpdateService &update,
const char *uri_utf8, bool discard)
{
unsigned ret = update.Enqueue(uri_utf8, discard);
r.Fmt(FMT_STRING("updating_db: {}\n"), ret);
r.Fmt("updating_db: {}\n", ret);
return CommandResult::OK;
}

Expand All @@ -259,7 +259,7 @@ handle_update(Response &r, Database &db,
{
unsigned id = db.Update(uri_utf8, discard);
if (id > 0) {
r.Fmt(FMT_STRING("updating_db: {}\n"), id);
r.Fmt("updating_db: {}\n", id);
return CommandResult::OK;
} else {
/* Database::Update() has returned 0 without setting
Expand Down Expand Up @@ -326,7 +326,7 @@ handle_getvol(Client &client, Request, Response &r)

const auto volume = partition.mixer_memento.GetVolume(partition.outputs);
if (volume >= 0)
r.Fmt(FMT_STRING("volume: {}\n"), volume);
r.Fmt("volume: {}\n", volume);

return CommandResult::OK;
}
Expand Down Expand Up @@ -391,7 +391,7 @@ handle_config(Client &client, [[maybe_unused]] Request args, Response &r)
const Storage *storage = client.GetStorage();
if (storage != nullptr) {
const auto path = storage->MapUTF8("");
r.Fmt(FMT_STRING("music_directory: {}\n"), path);
r.Fmt("music_directory: {}\n", path);
}
#endif

Expand All @@ -406,7 +406,7 @@ handle_idle(Client &client, Request args, Response &r)
unsigned event = idle_parse_name(i);
if (event == 0) {
r.FmtError(ACK_ERROR_ARG,
FMT_STRING("Unrecognized idle event: {}"),
"Unrecognized idle event: {}",
i);
return CommandResult::ERROR;
}
Expand Down
2 changes: 1 addition & 1 deletion src/command/PartitionCommands.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ CommandResult
handle_listpartitions(Client &client, Request, Response &r)
{
for (const auto &partition : client.GetInstance().partitions) {
r.Fmt(FMT_STRING("partition: {}\n"), partition.name);
r.Fmt("partition: {}\n", partition.name);
}

return CommandResult::OK;
Expand Down
Loading

0 comments on commit 6efba9f

Please sign in to comment.