Skip to content

Commit

Permalink
Don't wrap named arg in cref and clarify docs
Browse files Browse the repository at this point in the history
  • Loading branch information
vitaut committed Mar 14, 2021
1 parent b8ff3c1 commit 5a1127b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 36 deletions.
24 changes: 8 additions & 16 deletions include/fmt/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,37 +168,29 @@ class dynamic_format_arg_store
/**
\rst
Adds a reference to the argument into the dynamic store for later passing to
a formatting function. Supports named arguments wrapped in
``std::reference_wrapper`` via ``std::ref()``/``std::cref()``.
a formatting function.
**Example**::
fmt::dynamic_format_arg_store<fmt::format_context> store;
char str[] = "1234567890";
store.push_back(std::cref(str));
int a1_val{42};
auto a1 = fmt::arg("a1_", a1_val);
store.push_back(std::cref(a1));
// Changing str affects the output but only for string and custom types.
str[0] = 'X';
std::string result = fmt::vformat("{} and {a1_}");
assert(result == "X234567890 and 42");
char band[] = "Rolling Stones";
store.push_back(std::cref(band));
band[9] = 'c'; // Changing str affects the output.
std::string result = fmt::vformat("{}", store);
// result == "Rolling Scones"
\endrst
*/
template <typename T> void push_back(std::reference_wrapper<T> arg) {
static_assert(
detail::is_named_arg<typename std::remove_cv<T>::type>::value ||
need_copy<T>::value,
need_copy<T>::value,
"objects of built-in types and string views are always copied");
emplace_arg(arg.get());
}

/**
Adds named argument into the dynamic store for later passing to a formatting
function. ``std::reference_wrapper`` is supported to avoid copying of the
argument.
argument. The name is always stored by reference.
*/
template <typename T>
void push_back(const detail::named_arg<char_type, T>& arg) {
Expand Down
5 changes: 3 additions & 2 deletions include/fmt/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -1619,8 +1619,9 @@ inline auto make_args_checked(const S& format_str,

/**
\rst
Returns a named argument to be used in a formatting function. It should only
be used in a call to a formatting function.
Returns a named argument to be used in a formatting function.
It should only be used in a call to a formatting function or
`dynamic_format_arg_store::push_back`.
**Example**::
Expand Down
22 changes: 4 additions & 18 deletions test/core-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -469,24 +469,10 @@ TEST(FormatDynArgsTest, NamedStrings) {

TEST(FormatDynArgsTest, NamedArgByRef) {
fmt::dynamic_format_arg_store<fmt::format_context> store;

// Note: fmt::arg() constructs an object which holds a reference
// to its value. It's not an aggregate, so it doesn't extend the
// reference lifetime. As a result, it's a very bad idea passing temporary
// as a named argument value. Only GCC with optimization level >0
// complains about this.
//
// A real life usecase is when you have both name and value alive
// guarantee their lifetime and thus don't want them to be copied into
// storages.
int a1_val{42};
auto a1 = fmt::arg("a1_", a1_val);
store.push_back("abc");
store.push_back(1.5f);
store.push_back(std::cref(a1));

std::string result = fmt::vformat("{a1_} and {} and {} and {}", store);
EXPECT_EQ("42 and abc and 1.5 and 42", result);
char band[] = "Rolling Stones";
store.push_back(fmt::arg("band", std::cref(band)));
band[9] = 'c'; // Changing str affects the output.
EXPECT_EQ(fmt::vformat("{band}", store), "Rolling Scones");
}

TEST(FormatDynArgsTest, NamedCustomFormat) {
Expand Down

0 comments on commit 5a1127b

Please sign in to comment.