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

<format>: Make non-Standard constructors of basic_format_arg::handle and basic_format_context private #4489

Merged
merged 3 commits into from
Mar 19, 2024
Merged

<format>: Make non-Standard constructors of basic_format_arg::handle and basic_format_context private #4489

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3fdd373
Eliminate non-Standard public constructors for...
frederick-vs-ja Mar 18, 2024
87a61a1
Use `_STL_INTERNAL_STATIC_ASSERT`.
StephanTLavavej Mar 18, 2024
7fdaf34
`basic_format_context` shouldn't be default-constructible.
StephanTLavavej Mar 18, 2024
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
59 changes: 33 additions & 26 deletions stl/inc/format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,9 +708,7 @@ public:
private:
const void* _Ptr;
void(__cdecl* _Format)(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void*);
friend basic_format_arg;

public:
template <class _Ty>
explicit handle(_Ty& _Val) noexcept
: _Ptr(_STD addressof(_Val)),
Expand All @@ -719,27 +717,23 @@ public:
using _Td = remove_const_t<_Ty>;
// doesn't drop const-qualifier per an unnumbered LWG issue
using _Tq = conditional_t<_Formattable_with<const _Ty, _Context>, const _Ty, _Ty>;
if constexpr (_Formattable_with_non_const<_Tq, _Context>) {
static_assert(_Formattable_with<_Tq, _Context>,
"The format() member function can't be called on const formatter<T>. "
"To make the formatter usable, add const to format(). "
"See N4971 [format.arg]/12, [format.formattable], and [formatter.requirements].");
} else {
static_assert(_Formattable_with<_Tq, _Context>,
"Cannot format an argument. "
"To make this type formattable, provide a formatter<T> specialization. "
"See N4971 [format.arg]/12, [format.formattable], and [formatter.requirements].");
}
static_assert(_Formattable_with<_Tq, _Context>);
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

typename _Context::template formatter_type<_Td> _Formatter;
_Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx));
_Format_ctx.advance_to(
_Formatter.format(*const_cast<_Tq*>(static_cast<const _Td*>(_Ptr)), _Format_ctx));
}) {}

public:
void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) const {
_Format(_Parse_ctx, _Format_ctx, _Ptr);
}

template <class _Ty>
_NODISCARD static handle _Make_from(_Ty& _Val) noexcept {
return handle{_Val};
}
};

#if defined(__clang__) || defined(__EDG__) // TRANSITION, LLVM-81774 (Clang), VSO-1956558 (EDG)
Expand All @@ -755,7 +749,7 @@ public:
// Function template _Make_from mirrors the exposition-only single-argument constructor template of
// basic_format_arg (N4950 [format.arg]).
template <_Formattable_with<_Context> _Ty>
static basic_format_arg _Make_from(_Ty& _Val) noexcept {
_NODISCARD static basic_format_arg _Make_from(_Ty& _Val) noexcept {
using _Erased_type = _Format_arg_traits<_Context>::template _Storage_type<_Ty>;
if constexpr (is_same_v<remove_const_t<_Ty>, char> && is_same_v<_CharType, wchar_t>) {
return basic_format_arg(static_cast<_Erased_type>(static_cast<unsigned char>(_Val)));
Expand Down Expand Up @@ -892,7 +886,7 @@ auto _Format_arg_traits<_Context>::_Type_eraser() {
return static_cast<const void*>(nullptr);
} else {
int _Dummy{};
return typename basic_format_arg<_Context>::handle{_Dummy};
return basic_format_arg<_Context>::handle::_Make_from(_Dummy);
}
}

Expand Down Expand Up @@ -2051,6 +2045,8 @@ private:
if constexpr (is_same_v<remove_const_t<remove_reference_t<_Ty>>, char> && is_same_v<_CharType, wchar_t>) {
_Store_impl<_Erased_type>(
_Arg_index, _Arg_type, static_cast<_Erased_type>(static_cast<unsigned char>(_Val)));
} else if constexpr (is_same_v<_Erased_type, typename basic_format_arg<_Context>::handle>) {
_Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type::_Make_from(_Val));
}
#if !_HAS_CXX23
// Workaround towards N4950 [format.arg]/6.8 in C++20
Expand Down Expand Up @@ -2199,19 +2195,21 @@ private:
basic_format_args<basic_format_context> _Args;
_Lazy_locale _Loc;

constexpr basic_format_context(_Out&& _OutputIt_, const basic_format_args<basic_format_context>& _Ctx_args)
: _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args) {}

constexpr basic_format_context(
_Out&& _OutputIt_, const basic_format_args<basic_format_context>& _Ctx_args, const _Lazy_locale& _Loc_)
: _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args), _Loc(_Loc_) {}

public:
using iterator = _Out;
using char_type = _CharT;

template <class _Ty>
using formatter_type = formatter<_Ty, _CharT>;

constexpr basic_format_context(_Out _OutputIt_, basic_format_args<basic_format_context> _Ctx_args)
: _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args) {}

constexpr basic_format_context(
_Out _OutputIt_, basic_format_args<basic_format_context> _Ctx_args, const _Lazy_locale& _Loc_)
: _OutputIt(_STD move(_OutputIt_)), _Args(_Ctx_args), _Loc(_Loc_) {}
basic_format_context() = default;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

_NODISCARD basic_format_arg<basic_format_context> arg(size_t _Id) const noexcept {
return _Args.get(_Id);
Expand All @@ -2232,6 +2230,15 @@ public:
_NODISCARD _Lazy_locale _Get_lazy_locale() const {
return _Loc;
}

_NODISCARD static constexpr basic_format_context _Make_from(
_Out _OutputIt_, basic_format_args<basic_format_context> _Ctx_args) {
return basic_format_context{_STD move(_OutputIt_), _Ctx_args};
}
_NODISCARD static constexpr basic_format_context _Make_from(
_Out _OutputIt_, basic_format_args<basic_format_context> _Ctx_args, const _Lazy_locale& _Loc_) {
return basic_format_context{_STD move(_OutputIt_), _Ctx_args, _Loc_};
}
};

#if _HAS_CXX23
Expand Down Expand Up @@ -3540,7 +3547,7 @@ struct _Default_arg_formatter {

_OutputIt operator()(basic_format_arg<_Context>::handle _Handle) && {
basic_format_parse_context<_CharT> _Parse_ctx({});
basic_format_context<_OutputIt, _CharT> _Format_ctx(_STD move(_Out), _Args, _Loc);
auto _Format_ctx = _Context::_Make_from(_STD move(_Out), _Args, _Loc);
_Handle.format(_Parse_ctx, _Format_ctx);
return _Format_ctx.out();
}
Expand Down Expand Up @@ -3626,11 +3633,11 @@ struct _Format_handler {
_Context _Ctx;

explicit _Format_handler(_OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args)
: _Parse_context(_Str), _Ctx(_STD move(_Out), _Format_args) {}
: _Parse_context(_Str), _Ctx(_Context::_Make_from(_STD move(_Out), _Format_args)) {}

explicit _Format_handler(_OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args,
const _Lazy_locale& _Loc)
: _Parse_context(_Str), _Ctx(_STD move(_Out), _Format_args, _Loc) {}
: _Parse_context(_Str), _Ctx(_Context::_Make_from(_STD move(_Out), _Format_args, _Loc)) {}

void _On_text(const _CharT* _First, const _CharT* _Last) {
_Ctx.advance_to(_RANGES _Copy_unchecked(_First, _Last, _Ctx.out()).out);
Expand Down Expand Up @@ -4107,8 +4114,8 @@ protected:
}

basic_string<_CharT> _Tmp_buf;
basic_format_context<back_insert_iterator<basic_string<_CharT>>, _CharT> _Tmp_ctx{
_STD back_inserter(_Tmp_buf), {}, _Fmt_ctx._Get_lazy_locale()};
auto _Tmp_ctx = basic_format_context<back_insert_iterator<basic_string<_CharT>>, _CharT>::_Make_from(
_STD back_inserter(_Tmp_buf), {}, _Fmt_ctx._Get_lazy_locale());

_STD _Copy_unchecked(_Opening_bracket._Unchecked_begin(), _Opening_bracket._Unchecked_end(), _Tmp_ctx.out());
[&]<size_t... _Indices>(index_sequence<_Indices...>) {
Expand Down
47 changes: 41 additions & 6 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 @@ -233,19 +233,47 @@ void test_mixed_custom_formattable_type() {
test_custom_equiv_with_format_mixed<const void*, charT>(STR("{}{}"), nullptr);
}

// Test that handle doesn't have public non-Standard constructors.
template <class OutIt, class CharT>
void test_basic_format_arg_handle_construction() {
using handle = basic_format_arg<basic_format_context<OutIt, CharT>>::handle;

static_assert(is_constructible_v<handle, int&>);
static_assert(is_constructible_v<handle, const int&>);
static_assert(is_constructible_v<handle, handle>);
static_assert(is_constructible_v<handle, const handle&>);

static_assert(!is_constructible_v<handle, int&>);
static_assert(!is_constructible_v<handle, const int&>);
static_assert(!is_constructible_v<handle, int>);
static_assert(is_constructible_v<handle, const int>);
static_assert(!is_constructible_v<handle, const int>);

static_assert(is_constructible_v<handle, custom_formattable_type<CharT>&>);
static_assert(is_constructible_v<handle, const custom_formattable_type<CharT>&>);
static_assert(!is_constructible_v<handle, custom_formattable_type<CharT>&>);
static_assert(!is_constructible_v<handle, const custom_formattable_type<CharT>&>);
static_assert(!is_constructible_v<handle, custom_formattable_type<CharT>>);
static_assert(is_constructible_v<handle, const custom_formattable_type<CharT>>);
static_assert(!is_constructible_v<handle, const custom_formattable_type<CharT>>);
}

template <class T, class... Args>
constexpr bool is_constructible_with_trailing_empty_brace_impl = requires { T(declval<Args>()..., {}); };

static_assert(is_constructible_with_trailing_empty_brace_impl<string>);
static_assert(is_constructible_with_trailing_empty_brace_impl<pair<int, int>, int>);
static_assert(!is_constructible_with_trailing_empty_brace_impl<pair<int, int>, int, int>);

// Test that basic_format_context doesn't have public non-Standard constructors.
template <class OutIt, class CharT>
void test_basic_format_context_construction() {
using context = basic_format_context<OutIt, CharT>;

static_assert(is_default_constructible_v<context> == is_default_constructible_v<OutIt>);
static_assert(is_copy_constructible_v<context> == is_copy_constructible_v<OutIt>);
static_assert(is_move_constructible_v<context>);

static_assert(!is_constructible_v<context, OutIt, basic_format_args<context>>);
static_assert(!is_constructible_v<context, OutIt, const basic_format_args<context>&>);

static_assert(is_constructible_with_trailing_empty_brace_impl<context> == is_default_constructible_v<OutIt>);
static_assert(!is_constructible_with_trailing_empty_brace_impl<context, OutIt, basic_format_args<context>>);
static_assert(!is_constructible_with_trailing_empty_brace_impl<context, OutIt, const basic_format_args<context>&>);
}

int main() {
Expand All @@ -262,5 +290,12 @@ int main() {
test_basic_format_arg_handle_construction<wchar_t*, wchar_t>();
test_basic_format_arg_handle_construction<wstring::iterator, wchar_t>();
test_basic_format_arg_handle_construction<back_insert_iterator<wstring>, wchar_t>();

test_basic_format_context_construction<char*, char>();
test_basic_format_context_construction<string::iterator, char>();
test_basic_format_context_construction<back_insert_iterator<string>, char>();
test_basic_format_context_construction<wchar_t*, wchar_t>();
test_basic_format_context_construction<wstring::iterator, wchar_t>();
test_basic_format_context_construction<back_insert_iterator<wstring>, wchar_t>();
return 0;
}
Loading