Skip to content

Commit

Permalink
fix: fix build issue in std latest for consteval fmt constructors (Vo…
Browse files Browse the repository at this point in the history
…wpalWabbit#3862)

* fix: fix build issue in std latest for consteval fmt constructors

* fix invalid format string

* use views by value

* workaround to support more than 1 fmt version

* fix 3 more issues

* more fixes
  • Loading branch information
jackgerrits authored Apr 14, 2022
1 parent aaa1832 commit 69eee24
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 23 deletions.
2 changes: 1 addition & 1 deletion vowpalwabbit/cost_sensitive.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ void output_example(
if (cl.x < min) { min = cl.x; }
}
if (chosen_loss == FLT_MAX)
{ all.logger.err_warn("csoaa predicted an invalid class. Are all multi-class labels in the {1..k} range?"); }
{ all.logger.err_warn("csoaa predicted an invalid class. Are all multi-class labels in the {{1..k}} range?"); }

loss = (chosen_loss - min) * ec.weight;
// TODO(alberto): add option somewhere to allow using absolute loss instead?
Expand Down
47 changes: 43 additions & 4 deletions vowpalwabbit/io/include/vw/io/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#pragma once

#include "fmt/core.h"

#include <iostream>
#include <memory>
#include <string>
#include <utility>
Expand Down Expand Up @@ -281,50 +284,86 @@ struct logger
friend logger create_custom_sink_logger_legacy(void* context, logger_legacy_output_func_t func);

public:
#if FMT_VERSION >= 80000
template <typename... Args>
void err_info(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void err_info(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->err_info(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void err_warn(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void err_warn(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->err_warn(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void err_error(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void err_error(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->err_error(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void err_critical(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void err_critical(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->err_critical(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void out_info(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void out_info(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->out_info(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void out_warn(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void out_warn(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->out_warn(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void out_error(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void out_error(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->out_error(fmt, std::forward<Args>(args)...);
}

#if FMT_VERSION >= 80000
template <typename... Args>
void out_critical(fmt::format_string<Args...> fmt, Args&&... args)
#else
template <typename FormatString, typename... Args>
void out_critical(const FormatString& fmt, Args&&... args)
#endif
{
_logger_impl->out_critical(fmt, std::forward<Args>(args)...);
}
Expand Down
3 changes: 1 addition & 2 deletions vowpalwabbit/io/src/logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ void logger::log_summary()
{
if (_logger_impl->_max_limit != SIZE_MAX && _logger_impl->_log_count > _logger_impl->_max_limit)
{
_logger_impl->err_critical(
"Omitted some log lines. Re-run without --limit_output N for full log. Total log lines: {}",
err_critical("Omitted some log lines. Re-run without --limit_output N for full log. Total log lines: {}",
_logger_impl->_log_count);
}
}
Expand Down
12 changes: 6 additions & 6 deletions vowpalwabbit/learner.h
Original file line number Diff line number Diff line change
Expand Up @@ -703,17 +703,17 @@ struct reduction_learner_builder
if (in_pred_type != base_out_pred_type)
{
logger->err_warn(
fmt::format("Input prediction type: {} of reduction: {} does not match output prediction type: {} of base "
"reduction: {}.",
to_string(in_pred_type), this->_learner->name, to_string(base_out_pred_type),
this->_learner->learn_fd.base->get_name()));
"Input prediction type: {} of reduction: {} does not match output prediction type: {} of base "
"reduction: {}.",
to_string(in_pred_type), this->_learner->name, to_string(base_out_pred_type),
this->_learner->learn_fd.base->get_name());
}
if (out_label_type != base_in_label_type)
{
logger->err_warn(fmt::format(
logger->err_warn(
"Output label type: {} of reduction: {} does not match input label type: {} of base reduction: {}.",
to_string(out_label_type), this->_learner->name, to_string(base_in_label_type),
this->_learner->learn_fd.base->get_name()));
this->_learner->learn_fd.base->get_name());
}
}
return this->_learner;
Expand Down
8 changes: 7 additions & 1 deletion vowpalwabbit/model_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ size_t write_text_mode_output(io_buf& io, const T& var, const std::string& name_
std::string message;
// If the user has supplied a template string then use that.
if (name_or_readable_field_template.find("{}") != VW::string_view::npos)
{ message = fmt::format(name_or_readable_field_template, var); }
{
#if FMT_VERSION >= 80000
message = fmt::format(fmt::runtime(name_or_readable_field_template), var);
#else
message = fmt::format(name_or_readable_field_template, var);
#endif
}
else
{
// Use the default template string.
Expand Down
6 changes: 3 additions & 3 deletions vowpalwabbit/parse_args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ input_options parse_source(VW::workspace& all, options_i& options)

// We are done adding new options. Before we are allowed to get the positionals we need to check unregistered.
auto warnings = all.options->check_unregistered();
for (const auto& warning : warnings) { all.logger.err_warn(warning); }
for (const auto& warning : warnings) { all.logger.err_warn("{}", warning); }

// Check if the options provider has any positional args. Only really makes sense for command line, others just return
// an empty list.
Expand Down Expand Up @@ -718,8 +718,8 @@ void parse_feature_tweaks(options_i& options, VW::workspace& all, bool interacti
interactions_settings_duplicated /*settings were restored from model file to file_options and overriden by params from command line*/)
{
all.logger.err_warn(
"model file has set of {-q, --cubic, --interactions} settings stored, but they'll be "
"OVERRIDDEN by set of {-q, --cubic, --interactions} settings from command line.");
"model file has set of {{-q, --cubic, --interactions}} settings stored, but they'll be "
"OVERRIDDEN by set of {{-q, --cubic, --interactions}} settings from command line.");
// in case arrays were already filled in with values from old model file - reset them
if (!all.interactions.empty()) { all.interactions.clear(); }
}
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/parse_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class TC_parser
}
else
{
warn_logger.err_warn(ss.str());
warn_logger.err_warn("{}", ss.str());
}
}

Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/reductions/boosting.cc
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,10 @@ void save_load(boosting& o, io_buf& model_file, bool read, bool text)
else
{
fmt::format_to(
std::back_inserter(buffer), "Saving alpha, current weighted_examples = {)\n", o.all->sd->weighted_examples());
std::back_inserter(buffer), "Saving alpha, current weighted_examples = {}\n", o.all->sd->weighted_examples());
}

for (int i = 0; i < o.N; i++) { fmt::format_to(std::back_inserter(buffer), "{} \n", o.alpha[i]); }
for (int i = 0; i < o.N; i++) { fmt::format_to(std::back_inserter(buffer), "{}\n", o.alpha[i]); }
o.logger.err_info("{}", fmt::to_string(buffer));
}
}
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/reductions/bs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ base_learner* VW::reductions::bs_setup(VW::setup_base_i& stack_builder)
}
else
{
all.logger.err_warn("bs_type must be in {'mean','vote'}; resetting to mean.");
all.logger.err_warn("bs_type must be in {{'mean','vote'}}; resetting to mean.");
data->bs_type = BS_TYPE_MEAN;
}
}
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/reductions/cb/cb_adf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ VW::LEARNER::base_learner* VW::reductions::cb_adf_setup(VW::setup_base_i& stack_
catch (const VW::vw_exception& /*exception*/)
{
all.logger.err_warn(
"cb_type must be in {'ips','dr','mtr','dm','sm'}; resetting to mtr. Input was: '{}'", type_string);
"cb_type must be in {{'ips','dr','mtr','dm','sm'}}; resetting to mtr. Input was: '{}'", type_string);
cb_type = VW::cb_type_t::mtr;
}

Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/reductions/search/search.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2496,7 +2496,7 @@ void ensure_param(float& v, float lo, float hi, float def, const char* str, VW::
{
if ((v < lo) || (v > hi))
{
logger.err_warn(str);
logger.err_warn("{}", str);
v = def;
}
}
Expand Down

0 comments on commit 69eee24

Please sign in to comment.