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.

Sign up for GitHub

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

Jump to bottom

Implement P2905R2 Runtime Format Strings #4196

Merged
merged 5 commits into from
Jan 9, 2024
Merged

Implement P2905R2 Runtime Format Strings #4196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb0d2da
Implement P2905R2 Runtime Format Strings
frederick-vs-ja Nov 15, 2023
0e24b20
Handle `/permissive`
frederick-vs-ja Nov 15, 2023
4776a72
Merge branch 'main' into p2905r2
frederick-vs-ja Nov 17, 2023
7bb56b5
Merge branch 'main' into p2905r2
StephanTLavavej Dec 16, 2023
03c2b35
Code review feedback.
StephanTLavavej Dec 16, 2023
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
14 changes: 8 additions & 6 deletions stl/inc/format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3688,18 +3688,20 @@ _EXPORT_STD using format_args = basic_format_args<format_context>;
_EXPORT_STD using wformat_args = basic_format_args<wformat_context>;

_EXPORT_STD template <class _Context = format_context, class... _Args>
_NODISCARD auto make_format_args(_Args&&... _Vals) {
static_assert((_Formattable_with<remove_cvref_t<_Args>, _Context> && ...),
_NODISCARD auto make_format_args(_Args&... _Vals) {
// TRANSITION, should cite the new working draft
static_assert((_Formattable_with<remove_const_t<_Args>, _Context> && ...),
"Cannot format an argument. To make type T formattable, provide a formatter<T> specialization. "
"See N4950 [format.arg.store]/2 and [formatter.requirements].");
"See N4964 [format.arg.store]/2 (along with modification in P2905R2) and [formatter.requirements].");
return _Format_arg_store<_Context, _Args...>{_Vals...};
}

_EXPORT_STD template <class... _Args>
_NODISCARD auto make_wformat_args(_Args&&... _Vals) {
static_assert((_Formattable_with<remove_cvref_t<_Args>, wformat_context> && ...),
_NODISCARD auto make_wformat_args(_Args&... _Vals) {
// TRANSITION, should cite the new working draft
static_assert((_Formattable_with<remove_const_t<_Args>, wformat_context> && ...),
"Cannot format an argument. To make type T formattable, provide a formatter<T> specialization. "
"See N4950 [format.arg.store]/2 and [formatter.requirements].");
"See N4964 [format.arg.store]/2 (along with modification in P2905R2) and [formatter.requirements].");
return _Format_arg_store<wformat_context, _Args...>{_Vals...};
}

Expand Down
6 changes: 2 additions & 4 deletions stl/inc/ostream
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,11 +1244,9 @@ void _Print_impl(const _Add_newline _Add_nl, ostream& _Ostr, const format_string

if constexpr (_Has_format_args) {
if constexpr (_STD _Is_ordinary_literal_encoding_utf8()) {
_STD _Vprint_unicode_impl(
_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...));
_STD _Vprint_unicode_impl(_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_Args...));
} else {
_STD _Vprint_nonunicode_impl(
_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...));
_STD _Vprint_nonunicode_impl(_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_Args...));
}
} else {
const string _Unescaped_str{_Unescape_braces(_Add_nl, _Fmt.get())};
Expand Down
6 changes: 2 additions & 4 deletions stl/inc/print
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,9 @@ void _Print_impl(

if constexpr (_Has_format_args) {
if constexpr (_STD _Is_ordinary_literal_encoding_utf8()) {
_STD _Vprint_unicode_impl(
_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...));
_STD _Vprint_unicode_impl(_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_Args...));
} else {
_STD _Vprint_nonunicode_impl(
_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...));
_STD _Vprint_nonunicode_impl(_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_Args...));
}
} else {
const string _Unescaped_str{_Unescape_braces(_Add_nl, _Fmt.get())};
Expand Down
1 change: 1 addition & 0 deletions stl/inc/yvals_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@
// P2711R1 Making Multi-Param Constructors Of Views explicit
// P2736R2 Referencing The Unicode Standard
// P2770R0 Stashing Stashing Iterators For Proper Flattening
// P2905R2 Runtime Format Strings
// P2909R4 Fix Formatting Of Code Units As Integers

// _HAS_CXX20 indirectly controls:
Expand Down
4 changes: 0 additions & 4 deletions tests/libcxx/expected_results.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,6 @@ std/language.support/support.limits/support.limits.general/utility.version.compi
std/language.support/support.limits/support.limits.general/expected.version.compile.pass.cpp FAIL
std/language.support/support.limits/support.limits.general/mdspan.version.compile.pass.cpp FAIL

# P2905R2 Runtime Format Strings
std/utilities/format/format.arguments/format.arg.store/make_format_args.pass.cpp FAIL
std/utilities/format/format.arguments/format.arg.store/make_wformat_args.pass.cpp FAIL

# P2918R2 Runtime Format Strings II
std/utilities/format/format.fmt.string/ctor.runtime-format-string.pass.cpp FAIL
std/utilities/format/format.functions/format.locale.runtime_format.pass.cpp FAIL
Expand Down
4 changes: 2 additions & 2 deletions tests/std/include/test_format_support.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ struct VFormatFn {
template <class... Args>
[[nodiscard]] auto operator()(const std::basic_string_view<CharT> str, Args&&... args) const {
if constexpr (std::same_as<CharT, char>) {
return std::vformat(str, std::make_format_args(std::forward<Args>(args)...));
return std::vformat(str, std::make_format_args(args...));
} else {
return std::vformat(str, std::make_wformat_args(std::forward<Args>(args)...));
return std::vformat(str, std::make_wformat_args(args...));
}
}
};
Expand Down
6 changes: 3 additions & 3 deletions tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ bool test_parse_chrono_format_specs() {
}

template <class charT, class... Args>
auto make_testing_format_args(Args&&... vals) {
auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful
if constexpr (is_same_v<charT, wchar_t>) {
return make_wformat_args(forward<Args>(vals)...);
return make_wformat_args(vals...);
} else {
return make_format_args(forward<Args>(vals)...);
return make_format_args(vals...);
}
}

Expand Down
54 changes: 53 additions & 1 deletion tests/std/tests/P0645R10_text_formatting_args/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ void test_visit_monostate() {

template <class Context>
void test_lwg3810() {
[[maybe_unused]] auto args_store = make_format_args<Context>(1, 2, 3);
int args[]{1, 2, 3};
[[maybe_unused]] auto args_store = make_format_args<Context>(args[0], args[1], args[2]);
static_assert(same_as<decltype(basic_format_args{args_store}), basic_format_args<Context>>);
}

Expand All @@ -264,6 +265,54 @@ void test_lvalue_only_visitation() {
visit_format_arg(lvalue_only_visitor{}, basic_format_arg<Context>{});
}

namespace detail {
constexpr bool permissive() {
return false;
}

template <class>
struct DependentBase {
static constexpr bool permissive() {
return true;
}
};

template <class T>
struct Derived : DependentBase<T> {
static constexpr bool test() {
return permissive();
}
};
} // namespace detail
constexpr bool is_permissive = detail::Derived<int>::test();

template <class Context, class... Args>
concept CanMakeFormatArgs = requires(Args&&... args) { make_format_args<Context>(static_cast<Args&&>(args)...); };

// P2905R2 Runtime format strings (make make_(w)format_args only take lvalue references)
template <class Context>
void test_lvalue_reference_parameters() {
using char_type = Context::char_type;

static_assert(CanMakeFormatArgs<Context, int&, long long&, double&, char_type&, char_type*&, const char_type*&,
basic_string<char_type>&, basic_string_view<char_type>&>);
static_assert(
CanMakeFormatArgs<Context, const int&, const long long&, const double&, const char_type&, char_type* const&,
const char_type* const&, const basic_string<char_type>&, const basic_string_view<char_type>&>);

static_assert(CanMakeFormatArgs<Context, const int, const long long, const double, const char_type,
char_type* const, const char_type* const, const basic_string<char_type>, const basic_string_view<char_type>>);
Comment on lines +303 to +304
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: The fact that const rvalues are accepted indicates a design problem with the paper.


static_assert(!CanMakeFormatArgs<Context, int>);
static_assert(!CanMakeFormatArgs<Context, long long>);
static_assert(!CanMakeFormatArgs<Context, double>);
static_assert(!CanMakeFormatArgs<Context, char_type>);
static_assert(!CanMakeFormatArgs<Context, char_type*>);
static_assert(!CanMakeFormatArgs<Context, const char_type*>);
static_assert(CanMakeFormatArgs<Context, basic_string<char_type>> == is_permissive);
static_assert(CanMakeFormatArgs<Context, basic_string_view<char_type>> == is_permissive);
}

int main() {
test_basic_format_arg<format_context>();
test_basic_format_arg<wformat_context>();
Expand All @@ -277,4 +326,7 @@ int main() {

test_lvalue_only_visitation<format_context>();
test_lvalue_only_visitation<wformat_context>();

test_lvalue_reference_parameters<format_context>();
test_lvalue_reference_parameters<wformat_context>();
}
6 changes: 3 additions & 3 deletions tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ using namespace std;

// copied from the text_formatting_formatting test case
template <class charT, class... Args>
auto make_testing_format_args(Args&&... vals) {
auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful
if constexpr (is_same_v<charT, wchar_t>) {
return make_wformat_args(forward<Args>(vals)...);
return make_wformat_args(vals...);
} else {
return make_format_args(forward<Args>(vals)...);
return make_format_args(vals...);
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/std/tests/P0645R10_text_formatting_formatting/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ template <class CharT, class Alloc = allocator<CharT>>
using alternative_basic_string = basic_string<CharT, alternative_char_traits<CharT>, Alloc>;

template <class charT, class... Args>
auto make_testing_format_args(Args&&... vals) {
auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful
if constexpr (is_same_v<charT, wchar_t>) {
return make_wformat_args(forward<Args>(vals)...);
return make_wformat_args(vals...);
} else {
return make_format_args(forward<Args>(vals)...);
return make_format_args(vals...);
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/std/tests/P0645R10_text_formatting_utf8/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ void test_multibyte_format_strings() {
{
try {
// Bad fill character encoding: missing lead byte before \x9f
(void) vformat("{:\x9f\x8f\x88<10}"sv, make_format_args(42));
int arg = 42;
(void) vformat("{:\x9f\x8f\x88<10}"sv, make_format_args(arg));
assert(false);
} catch (const format_error&) {
}
Expand Down
6 changes: 3 additions & 3 deletions tests/std/tests/P2286R8_text_formatting_escaping/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ template <typename CharT>
#define STR(Literal) (choose_literal<CharT>(Literal, L##Literal))

template <class charT, class... Args>
auto make_testing_format_args(Args&&... vals) {
auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful
if constexpr (is_same_v<charT, wchar_t>) {
return make_wformat_args(forward<Args>(vals)...);
return make_wformat_args(vals...);
} else {
return make_format_args(forward<Args>(vals)...);
return make_format_args(vals...);
}
}

Expand Down