From ad0da1472eb4ec65b2166606d4fa7955338984d1 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Wed, 22 Dec 2021 00:44:44 +0700 Subject: [PATCH 1/6] format should work for volatile arguments --- stl/inc/format | 2 +- tests/std/tests/P0645R10_text_formatting_args/test.cpp | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 261488cdfae..8018069640f 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1459,7 +1459,7 @@ struct _Format_arg_traits { // clang-format on template - using _Storage_type = decltype(_Phony_basic_format_arg_constructor(_STD declval())); + using _Storage_type = decltype(_Phony_basic_format_arg_constructor(_STD declval&>())); template static constexpr size_t _Storage_size = sizeof(_Storage_type<_Ty>); diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 494fd95fc1b..b1487ac767a 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -217,6 +217,13 @@ void test_visit_monostate() { assert(visit_format_arg(visitor, basic_format_arg()) == Arg_type::none); } +void test_gh_2427() { + // : volatile integral compilation error + volatile int vol = 42; + auto ret = format("{}", vol); + assert(ret == "42"); +} + int main() { test_basic_format_arg(); test_basic_format_arg(); @@ -224,4 +231,6 @@ int main() { test_format_arg_store(); test_visit_monostate(); test_visit_monostate(); + + test_gh_2427(); } From 98a617464cbac9653a3255fc8ba04bc510940b8a Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 20 Jan 2022 17:32:24 +0700 Subject: [PATCH 2/6] cannot use memcpy with volatile types --- stl/inc/format | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 15d9e9c4788..f339dc21f85 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -374,7 +374,7 @@ _NODISCARD constexpr const _CharT* _Parse_nonnegative_integer( const _CharT* _First, const _CharT* _Last, unsigned int& _Value) { _STL_INTERNAL_CHECK(_First != _Last && '0' <= *_First && *_First <= '9'); - constexpr auto _Max_int = static_cast((numeric_limits::max)()); + constexpr auto _Max_int = static_cast((numeric_limits::max) ()); constexpr auto _Big_int = _Max_int / 10u; _Value = 0; @@ -1134,7 +1134,7 @@ template _NODISCARD constexpr int _Get_dynamic_specs(const _FormatArg _Arg) { _STL_INTERNAL_STATIC_ASSERT(_Is_any_of_v<_Handler, _Width_checker, _Precision_checker>); const unsigned long long _Val = _STD visit_format_arg(_Handler{}, _Arg); - if (_Val > static_cast((numeric_limits::max)())) { + if (_Val > static_cast((numeric_limits::max) ())) { _THROW(format_error("Number is too big.")); } @@ -1208,7 +1208,7 @@ private: _ParseContext& _Parse_ctx; _NODISCARD static constexpr int _Verify_dynamic_arg_index_in_range(const size_t _Idx) { - if (_Idx > static_cast((numeric_limits::max)())) { + if (_Idx > static_cast((numeric_limits::max) ())) { _THROW(format_error("Dynamic width or precision index too large.")); } @@ -1282,7 +1282,7 @@ public: template constexpr void _On_type(_CharT _Type) { - if (_Type < 0 || _Type > (numeric_limits::max)()) { + if (_Type < 0 || _Type > (numeric_limits::max) ()) { _THROW(format_error("Invalid type specification.")); } const char _Narrow_type = static_cast(_Type); @@ -1556,7 +1556,14 @@ private: _Arg_type = _Basic_format_arg_type::_Custom_type; } - _Store_impl<_Erased_type>(_Arg_index, _Arg_type, static_cast<_Erased_type>(_Val)); + if constexpr (is_volatile_v<_Ty>) { + // _Store_impl uses memcpy for storing the value. + // We cannot use memcpy with volatile types so make a temporary + const _Erased_type _Temp = _Val; + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Temp); + } else { + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, static_cast<_Erased_type>(_Val)); + } } public: @@ -2309,8 +2316,8 @@ _NODISCARD _OutputIt _Write_integral( if (_Separators > 0) { return _Write_separated_integer(_Buffer_start, _End, _Groups, - _STD use_facet>(_Locale._Get()).thousands_sep(), // - _Separators, _STD move(_Out)); + _STD use_facet>(_Locale._Get()).thousands_sep(), // + _Separators, _STD move(_Out)); } return _RANGES _Copy_unchecked(_Buffer_start, _End, _STD move(_Out)).out; }; @@ -2462,7 +2469,7 @@ _NODISCARD _OutputIt _Fmt_write( auto _Buffer_start = _Buffer; auto _Width = static_cast(_Result.ptr - _Buffer_start); - const auto _Is_negative = (_STD signbit)(_Value); + const auto _Is_negative = (_STD signbit) (_Value); if (_Is_negative) { // Remove the '-', it will be dealt with directly @@ -2478,7 +2485,7 @@ _NODISCARD _OutputIt _Fmt_write( _Exponent = static_cast(_CSTD toupper(_Exponent)); } - const auto _Is_finite = (_STD isfinite)(_Value); + const auto _Is_finite = (_STD isfinite) (_Value); auto _Append_decimal = false; auto _Exponent_start = _Result.ptr; @@ -2497,7 +2504,7 @@ _NODISCARD _OutputIt _Fmt_write( _Exponent_start = _It; } } - _Integral_end = (_STD min)(_Radix_point, _Exponent_start); + _Integral_end = (_STD min) (_Radix_point, _Exponent_start); if (_Specs._Alt && _Radix_point == _Result.ptr) { // TRANSITION, decimal point may be wider @@ -2635,7 +2642,7 @@ _NODISCARD const _CharT* _Measure_string_prefix(const basic_string_view<_CharT> const auto _Last = _Pos + _Value.size(); int _Estimated_width = 0; // the estimated width of [_Value.data(), _Pos) const _Fmt_codec<_CharT> _Codec; - constexpr auto _Max_int = (numeric_limits::max)(); + constexpr auto _Max_int = (numeric_limits::max) (); while (_Pos != _Last) { if (_Estimated_width == _Max_width && _Max_width >= 0) { From 12bf14753b0117ef96075e27e8626eaf01d54446 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 20 Jan 2022 17:34:54 +0700 Subject: [PATCH 3/6] clang-format --- stl/inc/format | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index f339dc21f85..caa3274d6c1 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -374,7 +374,7 @@ _NODISCARD constexpr const _CharT* _Parse_nonnegative_integer( const _CharT* _First, const _CharT* _Last, unsigned int& _Value) { _STL_INTERNAL_CHECK(_First != _Last && '0' <= *_First && *_First <= '9'); - constexpr auto _Max_int = static_cast((numeric_limits::max) ()); + constexpr auto _Max_int = static_cast((numeric_limits::max)()); constexpr auto _Big_int = _Max_int / 10u; _Value = 0; @@ -1134,7 +1134,7 @@ template _NODISCARD constexpr int _Get_dynamic_specs(const _FormatArg _Arg) { _STL_INTERNAL_STATIC_ASSERT(_Is_any_of_v<_Handler, _Width_checker, _Precision_checker>); const unsigned long long _Val = _STD visit_format_arg(_Handler{}, _Arg); - if (_Val > static_cast((numeric_limits::max) ())) { + if (_Val > static_cast((numeric_limits::max)())) { _THROW(format_error("Number is too big.")); } @@ -1208,7 +1208,7 @@ private: _ParseContext& _Parse_ctx; _NODISCARD static constexpr int _Verify_dynamic_arg_index_in_range(const size_t _Idx) { - if (_Idx > static_cast((numeric_limits::max) ())) { + if (_Idx > static_cast((numeric_limits::max)())) { _THROW(format_error("Dynamic width or precision index too large.")); } @@ -1282,7 +1282,7 @@ public: template constexpr void _On_type(_CharT _Type) { - if (_Type < 0 || _Type > (numeric_limits::max) ()) { + if (_Type < 0 || _Type > (numeric_limits::max)()) { _THROW(format_error("Invalid type specification.")); } const char _Narrow_type = static_cast(_Type); @@ -2316,8 +2316,8 @@ _NODISCARD _OutputIt _Write_integral( if (_Separators > 0) { return _Write_separated_integer(_Buffer_start, _End, _Groups, - _STD use_facet>(_Locale._Get()).thousands_sep(), // - _Separators, _STD move(_Out)); + _STD use_facet>(_Locale._Get()).thousands_sep(), // + _Separators, _STD move(_Out)); } return _RANGES _Copy_unchecked(_Buffer_start, _End, _STD move(_Out)).out; }; @@ -2469,7 +2469,7 @@ _NODISCARD _OutputIt _Fmt_write( auto _Buffer_start = _Buffer; auto _Width = static_cast(_Result.ptr - _Buffer_start); - const auto _Is_negative = (_STD signbit) (_Value); + const auto _Is_negative = (_STD signbit)(_Value); if (_Is_negative) { // Remove the '-', it will be dealt with directly @@ -2485,7 +2485,7 @@ _NODISCARD _OutputIt _Fmt_write( _Exponent = static_cast(_CSTD toupper(_Exponent)); } - const auto _Is_finite = (_STD isfinite) (_Value); + const auto _Is_finite = (_STD isfinite)(_Value); auto _Append_decimal = false; auto _Exponent_start = _Result.ptr; @@ -2504,7 +2504,7 @@ _NODISCARD _OutputIt _Fmt_write( _Exponent_start = _It; } } - _Integral_end = (_STD min) (_Radix_point, _Exponent_start); + _Integral_end = (_STD min)(_Radix_point, _Exponent_start); if (_Specs._Alt && _Radix_point == _Result.ptr) { // TRANSITION, decimal point may be wider @@ -2642,7 +2642,7 @@ _NODISCARD const _CharT* _Measure_string_prefix(const basic_string_view<_CharT> const auto _Last = _Pos + _Value.size(); int _Estimated_width = 0; // the estimated width of [_Value.data(), _Pos) const _Fmt_codec<_CharT> _Codec; - constexpr auto _Max_int = (numeric_limits::max) (); + constexpr auto _Max_int = (numeric_limits::max)(); while (_Pos != _Last) { if (_Estimated_width == _Max_width && _Max_width >= 0) { From 5ae810a569dfc94eb01280332515dd55785db68f Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 20 Jan 2022 17:58:12 +0700 Subject: [PATCH 4/6] more precise comment --- stl/inc/format | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index caa3274d6c1..58a6d2256f6 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1557,8 +1557,9 @@ private: } if constexpr (is_volatile_v<_Ty>) { - // _Store_impl uses memcpy for storing the value. - // We cannot use memcpy with volatile types so make a temporary + // _Erased_type is not a volatile type, so static_cast<_Erased_type> would cast away volatile + // _Store_impl uses memcpy for storing the value. We cannot use memcpy with volatile types. + // Because of the two reasons we have to make a temporary const _Erased_type _Temp = _Val; _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Temp); } else { From 096603dcfc43e5ded9a18858cc1d267ac955e86c Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Thu, 20 Jan 2022 18:04:34 +0700 Subject: [PATCH 5/6] add static assert --- stl/inc/format | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/format b/stl/inc/format index 58a6d2256f6..5ec2190c593 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1560,6 +1560,7 @@ private: // _Erased_type is not a volatile type, so static_cast<_Erased_type> would cast away volatile // _Store_impl uses memcpy for storing the value. We cannot use memcpy with volatile types. // Because of the two reasons we have to make a temporary + static_assert(!is_volatile_v<_Erased_type>); const _Erased_type _Temp = _Val; _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Temp); } else { From 186a1216d1a601e7f85e74eba898f0c04a9d8a0c Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Wed, 26 Jan 2022 20:37:04 +0700 Subject: [PATCH 6/6] _Phony_basic_format_arg_constructor can work with volatile types --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 9691e4bfe31..738eb9a279c 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1471,7 +1471,7 @@ struct _Format_arg_traits { // clang-format on template - using _Storage_type = decltype(_Phony_basic_format_arg_constructor(_STD declval>())); + using _Storage_type = decltype(_Phony_basic_format_arg_constructor(_STD declval<_Ty>())); template static constexpr size_t _Storage_size = sizeof(_Storage_type>);