Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support named args in dynamic_format_arg_store (#1655). #1663

Merged
merged 28 commits into from
May 9, 2020
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7a1a67f
Dynamic arguments storage. Implementation of enhancement from issue
vsolontsov-ll Mar 9, 2020
833bfe3
Finxing build issues. (Not complete)
vsolontsov-ll Mar 9, 2020
c3c6d0c
Fixed a lifetime issue in test and minor build fixes.
vsolontsov-ll Mar 10, 2020
448fb74
MSVC2015 fix
vsolontsov-ll Mar 10, 2020
5930040
Clang-format applied.
vsolontsov-ll Mar 10, 2020
7433fd5
Merge branch 'master' of https://github.com/fmtlib/fmt.git
vsolontsov-ll Mar 12, 2020
f0984c4
Addressed minor issues, removed named_arg support.
vsolontsov-ll Mar 12, 2020
107f9a8
Custom list -- first cut.
vsolontsov-ll Mar 12, 2020
4351334
Update dyn-args.h
vsolontsov-ll Mar 12, 2020
90219a7
Build fixes
vsolontsov-ll Mar 13, 2020
bee2b5f
Preliminary cleanup.
vsolontsov-ll Mar 14, 2020
c2c5c5c
Merge branch 'master' of https://github.com/fmtlib/fmt.git
vsolontsov-ll Mar 14, 2020
6e047c2
minor (unused alias in test)
vsolontsov-ll Mar 14, 2020
70bc839
merge dyn-args.h to core.h
vsolontsov-ll Mar 14, 2020
def562c
minor fixes
vsolontsov-ll Mar 14, 2020
49a46f8
Removed conditional compilation as there's already the
vsolontsov-ll Mar 14, 2020
863aa63
trying to fix MSVC
vsolontsov-ll Mar 14, 2020
473ba57
Fixed comments
vsolontsov-ll Mar 14, 2020
ae512cd
Fixes after review
vsolontsov-ll Mar 16, 2020
baae799
Merge remote-tracking branch 'zverovich/master'
vsolontsov-ll May 3, 2020
c902f34
Support named args in dynamic_format_arg_store (#1655).
vsolontsov-ll May 3, 2020
efbcb27
fix build: exceptions and cast...
vsolontsov-ll May 3, 2020
e46c0a4
Fix namespace issue around exceptions marcos for MSVC
vsolontsov-ll May 4, 2020
6199e2a
Named arguments. Better wupport for std::reference_wrapper.
vsolontsov-ll May 4, 2020
ced27f1
Fix review comments
vsolontsov-ll May 7, 2020
d2a2c52
Fix gcc 4.8 build
vsolontsov-ll May 7, 2020
5f6f93b
Minor fixes.
vsolontsov-ll May 7, 2020
00b6883
Review fixes.
vsolontsov-ll May 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 146 additions & 48 deletions include/fmt/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ using wparse_context FMT_DEPRECATED_ALIAS = basic_format_parse_context<wchar_t>;

template <typename Context> class basic_format_arg;
template <typename Context> class basic_format_args;
template <typename Context> class dynamic_format_arg_store;

// A formatter for objects of type T.
template <typename T, typename Char = char, typename Enable = void>
Expand Down Expand Up @@ -1131,6 +1132,7 @@ template <typename Context> class basic_format_arg {

friend class basic_format_args<Context>;
friend class internal::arg_map<Context>;
friend class dynamic_format_arg_store<Context>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, basic_format_arg is constructed from dynamic_format_arg_store::emplace_arg() and needs to access private ctor. Also there's a need to update named_args with new size and pointer to array of named_arg_info after potential relocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


using char_type = typename Context::char_type;

Expand Down Expand Up @@ -1273,6 +1275,13 @@ template <typename T> struct is_reference_wrapper : std::false_type {};
template <typename T>
struct is_reference_wrapper<std::reference_wrapper<T>> : std::true_type {};

template <typename T> struct unwrap_reference { using type = T; };
template <typename T> struct unwrap_reference<std::reference_wrapper<T>> {
using type = T&;
};
template <typename T>
using unwrap_reference_t = typename unwrap_reference<T>::type;

class dynamic_arg_list {
// Workaround for clang's -Wweak-vtables. Unlike for regular classes, for
// templates it doesn't complain about inability to deduce single translation
Expand Down Expand Up @@ -1419,6 +1428,77 @@ inline format_arg_store<Context, Args...> make_format_args(
return {args...};
}

namespace internal {
template <typename Char> struct named_arg_base {
const Char* name;

// Serialized value<context>.
mutable char data[sizeof(basic_format_arg<buffer_context<Char>>)];

named_arg_base(const Char* nm) : name(nm) {}

template <typename Context> basic_format_arg<Context> deserialize() const {
basic_format_arg<Context> arg;
std::memcpy(&arg, data, sizeof(basic_format_arg<Context>));
return arg;
}
};

struct view {};

template <typename T, typename Char>
struct named_arg : view, named_arg_base<Char> {
const T& value;

named_arg(const Char* name, const T& val)
: named_arg_base<Char>(name), value(val) {}
};

// scope_exit is proposed in TS v3. This implementation is slightly different
// because without deduction guide there's no easy way to construct from lambda.
// Factory template function is needed (see make_scope_exit).
// Move assignment is disabled because of unclear semantic: whether to call
// currently assigned functor or not...
template <typename F> class scope_exit {
bool passive_{false};
F func_;

public:
explicit scope_exit(F&& f) noexcept : func_(std::forward<F>(f)) {}
~scope_exit() {
if (!passive_) func_();
}
scope_exit(const F&) = delete;
scope_exit(scope_exit&& rhs) noexcept : func_{std::move(rhs.func_)} {
rhs.release();
}
const scope_exit& operator=(const scope_exit&) = delete;
const scope_exit& operator=(scope_exit&&) = delete;
void release() noexcept { passive_ = true; }
};

template <typename F> scope_exit<F> make_scope_exit(F&& f) {
return scope_exit<F>{std::forward<F>(f)};
}

} // namespace internal

/**
\rst
Returns a named argument to be used in a formatting function. It should only
be used in a call to a formatting function.

**Example**::

fmt::print("Elapsed time: {s:.2f} seconds", fmt::arg("s", 1.23));
\endrst
*/
template <typename Char, typename T>
inline internal::named_arg<T, Char> arg(const Char* name, const T& arg) {
static_assert(!internal::is_named_arg<T>(), "nested named arguments");
return {name, arg};
}

/**
\rst
A dynamic version of `fmt::format_arg_store<>`.
Expand Down Expand Up @@ -1449,8 +1529,7 @@ class dynamic_format_arg_store
std::is_same<T, internal::std_string_view<char_type>>::value ||
(mapped_type != internal::type::cstring_type &&
mapped_type != internal::type::string_type &&
mapped_type != internal::type::custom_type &&
mapped_type != internal::type::named_arg_type))
mapped_type != internal::type::custom_type))
};
};

Expand All @@ -1460,6 +1539,7 @@ class dynamic_format_arg_store

// Storage of basic_format_arg must be contiguous.
std::vector<basic_format_arg<Context>> data_;
std::vector<internal::named_arg_info<char_type>> named_info_;

// Storage of arguments not fitting into basic_format_arg must grow
// without relocation because items in data_ refer to it.
Expand All @@ -1468,13 +1548,38 @@ class dynamic_format_arg_store
friend class basic_format_args<Context>;

unsigned long long get_types() const {
return internal::is_unpacked_bit | data_.size();
return internal::is_unpacked_bit | data_.size() |
(named_info_.empty() ? 0ULL
: static_cast<unsigned long long>(
internal::has_named_args_bit));
}

const basic_format_arg<Context>* data() const {
return named_info_.empty() ? data_.data() : data_.data() + 1;
}

template <typename T> void emplace_arg(const T& arg) {
data_.emplace_back(internal::make_arg<Context>(arg));
}

template <typename T>
void emplace_arg(const internal::named_arg<T, char_type>& arg) {
if (named_info_.empty()) {
data_.insert(
data_.begin(),
{static_cast<const internal::named_arg_info<char_type>*>(nullptr),
vsolontsov-ll marked this conversation as resolved.
Show resolved Hide resolved
0});
}
data_.emplace_back(internal::make_arg<Context>(
static_cast<const internal::unwrap_reference_t<T>&>(arg.value)));
vsolontsov-ll marked this conversation as resolved.
Show resolved Hide resolved

auto guard =
internal::make_scope_exit([this]() { this->data_.pop_back(); });
vsolontsov-ll marked this conversation as resolved.
Show resolved Hide resolved
named_info_.push_back({arg.name, static_cast<int>(data_.size() - 2u)});
data_[0].value_.named_args = {named_info_.data(), named_info_.size()};
guard.release();
}

public:
/**
\rst
Expand All @@ -1500,19 +1605,53 @@ class dynamic_format_arg_store
if (internal::const_check(need_copy<T>::value))
emplace_arg(dynamic_args_.push<stored_type<T>>(arg));
else
emplace_arg(arg);
emplace_arg(static_cast<const internal::unwrap_reference_t<T>&>(arg));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Then unwrap_reference_t can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

/**
\rst
Adds a reference to the argument into the dynamic store for later passing to
a formating function.
a formating function. Supports named arguments wrapped in
std::reference_wrapper (via std::ref()/std::cref()).

**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");
\endrst
*/
template <typename T> void push_back(std::reference_wrapper<T> arg) {
static_assert(
need_copy<T>::value,
internal::is_named_arg<typename std::decay<T>::type>::value ||
vsolontsov-ll marked this conversation as resolved.
Show resolved Hide resolved
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 formating
function. std::reference_wrapper is supported to avoid copying of the
argument.
*/
template <typename T>
void push_back(const internal::named_arg<T, char_type>& arg) {
const char_type* arg_name =
dynamic_args_.push<std::basic_string<char_type>>(arg.name).c_str();
if (internal::const_check(need_copy<T>::value)) {
emplace_arg(
fmt::arg(arg_name, dynamic_args_.push<stored_type<T>>(arg.value)));
} else
emplace_arg(fmt::arg(arg_name, arg.value));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: please wrap the else condition in a compound statement ({}) for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
};

/**
Expand Down Expand Up @@ -1597,7 +1736,7 @@ template <typename Context> class basic_format_args {
\endrst
*/
FMT_INLINE basic_format_args(const dynamic_format_arg_store<Context>& store)
: basic_format_args(store.get_types(), store.data_.data()) {}
: basic_format_args(store.get_types(), store.data()) {}

/**
\rst
Expand Down Expand Up @@ -1659,31 +1798,6 @@ template <typename Container>
struct is_contiguous_back_insert_iterator<std::back_insert_iterator<Container>>
: is_contiguous<Container> {};

template <typename Char> struct named_arg_base {
const Char* name;

// Serialized value<context>.
mutable char data[sizeof(basic_format_arg<buffer_context<Char>>)];

named_arg_base(const Char* nm) : name(nm) {}

template <typename Context> basic_format_arg<Context> deserialize() const {
basic_format_arg<Context> arg;
std::memcpy(&arg, data, sizeof(basic_format_arg<Context>));
return arg;
}
};

struct view {};

template <typename T, typename Char>
struct named_arg : view, named_arg_base<Char> {
const T& value;

named_arg(const Char* name, const T& val)
: named_arg_base<Char>(name), value(val) {}
};

// Reports a compile-time error if S is not a valid format string.
template <typename..., typename S, FMT_ENABLE_IF(!is_compile_string<S>::value)>
FMT_INLINE void check_format_string(const S&) {
Expand Down Expand Up @@ -1727,22 +1841,6 @@ inline void vprint_mojibake(std::FILE*, string_view, format_args) {}
#endif
} // namespace internal

/**
\rst
Returns a named argument to be used in a formatting function. It should only
be used in a call to a formatting function.

**Example**::

fmt::print("Elapsed time: {s:.2f} seconds", fmt::arg("s", 1.23));
\endrst
*/
template <typename Char, typename T>
inline internal::named_arg<T, Char> arg(const Char* name, const T& arg) {
static_assert(!internal::is_named_arg<T>(), "nested named arguments");
return {name, arg};
}

/** Formats a string and writes the output to ``out``. */
// GCC 8 and earlier cannot handle std::back_insert_iterator<Container> with
// vformat_to<ArgFormatter>(...) overload, so SFINAE on iterator type instead.
Expand Down
60 changes: 60 additions & 0 deletions test/core-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,66 @@ TEST(FormatDynArgsTest, CustomFormat) {
EXPECT_EQ("cust=0 and cust=1 and cust=3", result);
}

TEST(FormatDynArgsTest, NamedInt) {
fmt::dynamic_format_arg_store<fmt::format_context> store;
store.push_back(fmt::arg("a1", 42));
std::string result = fmt::vformat("{a1}", store);
EXPECT_EQ("42", result);
}

TEST(FormatDynArgsTest, NamedStrings) {
fmt::dynamic_format_arg_store<fmt::format_context> store;
char str[]{"1234567890"};
store.push_back(fmt::arg("a1", str));
store.push_back(fmt::arg("a2", std::cref(str)));
str[0] = 'X';

std::string result = fmt::vformat(
"{a1} and {a2}",
store);

EXPECT_EQ("1234567890 and X234567890", result);
}

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);
}

TEST(FormatDynArgsTest, NamedCustomFormat) {
fmt::dynamic_format_arg_store<fmt::format_context> store;
custom_type c{};
store.push_back(fmt::arg("c1", c));
++c.i;
store.push_back(fmt::arg("c2", c));
++c.i;
store.push_back(fmt::arg("c_ref", std::cref(c)));
++c.i;

std::string result = fmt::vformat("{c1} and {c2} and {c_ref}", store);
EXPECT_EQ("cust=0 and cust=1 and cust=3", result);
}

struct copy_throwable {
copy_throwable() {}
copy_throwable(const copy_throwable&) { throw "deal with it"; }
Expand Down