Add support for dynamic arg sets

[MikePopoloski]

This allows construction of basic_format_args from a dynamic set of arguments. The syntax is a little clunky and could probably be improved but this at least enables the functionality.

[MikePopoloski]

Related to issue #814

[vitaut]

Thanks for the PR. It looks good, just one minor comment.

[vitaut]

I don't think the cast is necessary here. Also please restring the line width to 80 chars.

[MikePopoloski]

The cast is necessary; the size is being truncated. On MSVC for example:
warning C4267: 'argument': conversion from 'size_t' to 'fmt::v5::basic_format_args<fmt::v5::format_context>::size_type', possible loss of data

I can change it to a static_cast if that makes you happier.

[do-m-en]

@vitaut This patch doesn't work as expected.

using ctx = fmt::format_context;
std::vector<fmt::basic_format_arg<ctx>> args;
args.emplace_back(fmt::internal::make_arg<ctx>(42));
args.emplace_back(fmt::internal::make_arg<ctx>(43));

std::cout << fmt::vformat("{} {}", fmt::basic_format_args<ctx>(args.data(), static_cast<unsigned int>(args.size()))) << '\n';

std::cout << fmt::vformat("{} {}", fmt::basic_format_args<ctx>(args.data(), static_cast<unsigned int>(888))) << '\n' << std::flush;

result is 42 43 in both cases so the size is ignored and only the count of {} in template string is used.

In the above case it's not relevant (just a bit more typing as it's just a tag to call a different constructor...) but this causes issues with named arguments:

using ctx = fmt::format_context;
std::vector<fmt::basic_format_arg<ctx>> args;
int fourty_two = 42;
std::string and_the_name = "content";
auto content = fmt::arg(and_the_name, fourty_two);
args.emplace_back(fmt::internal::make_arg<ctx>(content));
int fourty_three = 43;
std::string and_the_name_2 = "x2";
auto x2 = fmt::arg(and_the_name_2, fourty_three);
args.emplace_back(fmt::internal::make_arg<ctx>(x2));

std::cout << fmt::vformat("{content} {x2}", fmt::basic_format_args<ctx>(args.data(), static_cast<unsigned int>(args.size()))) << '\n' << std::flush;

Patch introduces types_(-(int64_t)count) which causes basic_format_args::type to return 15 (enum type::custom_type) for the 3rd parameter even though there are only 2 present.

Asan triggers in format.h:1449 once it tries to dereference args.args_[2].

The fix for this case is to just add the terminator but it's confusing since args.size() was already provided:

args.emplace_back(fmt::internal::make_arg<ctx>(x2));
args.emplace_back(fmt::basic_format_arg<ctx>()); // add the terminator...

std::cout << fmt::vformat("{content} {x2}", fmt::basic_format_args<ctx>(args.data(), static_cast<unsigned int>(args.size()))) << '\n' << std::flush;

[do-m-en]

The basic_format_args::type is not used on line 1449 - I've just changed the line from args.args_[i].type_ to args.type(i) for testing... The args.args_[i] triggers asan, args.type(i) start cycling as it always gets some non 0 (none_type) result. Sry that confusing part - the rest is as I've written.

[do-m-en]

Just for completeness:

std::cout << fmt::vformat("{} {} {}", fmt::basic_format_args<ctx>(args.data(), static_cast<unsigned int>(888))) << '\n' << std::flush;

Returns 42 42 43 which proves that the size parameter is not being used.

And also as a side note I'd expect the output to be 42 43 43 - repeating the last not the first item which makes me wonder if the {} are scanned frist and replaced later instead of just streaming and replacing on the fly when encountered in case of the dynamic vformat version (I could be wrong but my guess would be that it performs slower because of that).

[vitaut]

@MikePopoloski, could you please review the updated CONTIBUTING document, particularly the part about licensing, and let me know if you agree with it being applied to your contributions to {fmt}? The library is likely to be relicensed (#1073) so I'm collecting approval from all earlier contributors. Thanks!

[anxieux]

Just curious: I don't see any way to achieve this with the std::format (according to the C++ spec). Were there any discussions about standardizing this?

[vitaut]

There was no such discussion.

[MikePopoloski]

Looking back at this thread, it seems like I never responded to @vitaut 's question above? If so I sincerely apologize, and if still relevant I agree with whatever licensing changes you want to make. Sorry!