diff --git a/libcxx/docs/Status/Cxx2bIssues.csv b/libcxx/docs/Status/Cxx2bIssues.csv --- a/libcxx/docs/Status/Cxx2bIssues.csv +++ b/libcxx/docs/Status/Cxx2bIssues.csv @@ -250,7 +250,7 @@ "`3032 `__","``ValueSwappable`` requirement missing for ``push_heap`` and ``make_heap``","February 2023","","","" "`3085 `__","``char_traits::copy`` precondition too weak","February 2023","","","" "`3664 `__","`LWG 3392 `__ ``broke std::ranges::distance(a, a+3)``","February 2023","","","|ranges|" -"`3720 `__","Restrict the valid types of ``arg-id`` for width and precision in ``std-format-spec``","February 2023","","","|format|" +"`3720 `__","Restrict the valid types of ``arg-id`` for width and precision in ``std-format-spec``","February 2023","|Complete|","17.0","|format|" "`3756 `__","Is the ``std::atomic_flag`` class signal-safe?","February 2023","","","" "`3769 `__","``basic_const_iterator::operator==`` causes infinite constraint recursion","February 2023","","","|spaceship|" "`3807 `__","The feature test macro for ``ranges::find_last`` should be renamed","February 2023","","","|ranges|" diff --git a/libcxx/include/__format/format_functions.h b/libcxx/include/__format/format_functions.h --- a/libcxx/include/__format/format_functions.h +++ b/libcxx/include/__format/format_functions.h @@ -149,41 +149,44 @@ size_t __size_; }; -_LIBCPP_HIDE_FROM_ABI -constexpr void __compile_time_validate_integral(__arg_t __type) { - switch (__type) { - case __arg_t::__int: - case __arg_t::__long_long: - case __arg_t::__i128: - case __arg_t::__unsigned: - case __arg_t::__unsigned_long_long: - case __arg_t::__u128: - return; - - default: - std::__throw_format_error("Argument isn't an integral type"); - } -} - +// [format.string.std]/8 +// If { arg-idopt } is used in a width or precision, the value of the +// corresponding formatting argument is used in its place. If the +// corresponding formatting argument is not of standard signed or unsigned +// integer type, or its value is negative for precision or non-positive for +// width, an exception of type format_error is thrown. +// // _HasPrecision does the formatter have a precision? template -_LIBCPP_HIDE_FROM_ABI constexpr void -__compile_time_validate_argument(basic_format_parse_context<_CharT>& __parse_ctx, - __compile_time_basic_format_context<_CharT>& __ctx) { +_LIBCPP_HIDE_FROM_ABI constexpr void __compile_time_validate_argument( + basic_format_parse_context<_CharT>& __parse_ctx, __compile_time_basic_format_context<_CharT>& __ctx) { + auto __validate_type = [](__arg_t __type) { + // LWG3720 originally allowed "signed or unsigned integer types", however + // the final version explicitly changed it to "*standard* signed or unsigned + // integer types". It's trivial to use 128-bit integrals in libc++'s + // implementation, but other implementations may not implement it. + // (Using a width or precision, that does not fit in 64-bits, sounds very + // unlikely in real world code.) + switch (__type) { + case __arg_t::__int: + case __arg_t::__long_long: + case __arg_t::__unsigned: + case __arg_t::__unsigned_long_long: + return; + + default: + std::__throw_format_error("Replacement argument isn't a standard signed or unsigned integer type"); + } + }; + formatter<_Tp, _CharT> __formatter; __parse_ctx.advance_to(__formatter.parse(__parse_ctx)); - // [format.string.std]/7 - // ... If the corresponding formatting argument is not of integral type, or - // its value is negative for precision or non-positive for width, an - // exception of type format_error is thrown. - // - // Validate whether the arguments are integrals. if (__formatter.__parser_.__width_as_arg_) - __format::__compile_time_validate_integral(__ctx.arg(__formatter.__parser_.__width_)); + __validate_type(__ctx.arg(__formatter.__parser_.__width_)); if constexpr (_HasPrecision) if (__formatter.__parser_.__precision_as_arg_) - __format::__compile_time_validate_integral(__ctx.arg(__formatter.__parser_.__precision_)); + __validate_type(__ctx.arg(__formatter.__parser_.__precision_)); } // This function is not user facing, so it can directly use the non-standard types of the "variant". diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h --- a/libcxx/include/__format/parser_std_format_spec.h +++ b/libcxx/include/__format/parser_std_format_spec.h @@ -81,22 +81,33 @@ return _VSTD::__visit_format_arg( [](auto __arg) -> uint32_t { using _Type = decltype(__arg); - if constexpr (integral<_Type>) { + if constexpr (same_as<_Type, monostate>) + std::__throw_format_error("Argument index out of bounds"); + + // [format.string.std]/8 + // If { arg-idopt } is used in a width or precision, the value of the + // corresponding formatting argument is used in its place. If the + // corresponding formatting argument is not of standard signed or unsigned + // integer type, or its value is negative for precision or non-positive for + // width, an exception of type format_error is thrown. + // + // When an integral is used in a format function, it is stored as one of + // the types checked below. Other integral types are promoted. For example, + // a signed char is stored as an int. + if constexpr (same_as<_Type, int> || same_as<_Type, unsigned int> || // + same_as<_Type, long long> || same_as<_Type, unsigned long long>) { if constexpr (signed_integral<_Type>) { if (__arg < 0) std::__throw_format_error("A format-spec arg-id replacement shouldn't have a negative value"); } using _CT = common_type_t<_Type, decltype(__format::__number_max)>; - if (static_cast<_CT>(__arg) > - static_cast<_CT>(__format::__number_max)) + if (static_cast<_CT>(__arg) > static_cast<_CT>(__format::__number_max)) std::__throw_format_error("A format-spec arg-id replacement exceeds the maximum supported value"); return __arg; - } else if constexpr (same_as<_Type, monostate>) - std::__throw_format_error("Argument index out of bounds"); - else - std::__throw_format_error("A format-spec arg-id replacement argument isn't an integral type"); + } else + std::__throw_format_error("Replacement argument isn't a standard signed or unsigned integer type"); }, __format_arg); } diff --git a/libcxx/test/std/utilities/format/format.functions/format_tests.h b/libcxx/test/std/utilities/format/format.functions/format_tests.h --- a/libcxx/test/std/utilities/format/format.functions/format_tests.h +++ b/libcxx/test/std/utilities/format/format.functions/format_tests.h @@ -154,8 +154,8 @@ check_exception("A format-spec arg-id replacement exceeds the maximum supported value", SV("hello {:{}}"), world, unsigned(-1)); check_exception("Argument index out of bounds", SV("hello {:{}}"), world); - check_exception("A format-spec arg-id replacement argument isn't an integral type", SV("hello {:{}}"), world, - universe); + check_exception( + "Replacement argument isn't a standard signed or unsigned integer type", SV("hello {:{}}"), world, universe); check_exception("Using manual argument numbering in automatic argument numbering mode", SV("hello {:{0}}"), world, 1); check_exception("Using automatic argument numbering in manual argument numbering mode", SV("hello {0:{}}"), world, 1); // Arg-id may not have leading zeros. @@ -178,8 +178,8 @@ check_exception("A format-spec arg-id replacement exceeds the maximum supported value", SV("hello {:.{}}"), world, ~0u); check_exception("Argument index out of bounds", SV("hello {:.{}}"), world); - check_exception("A format-spec arg-id replacement argument isn't an integral type", SV("hello {:.{}}"), world, - universe); + check_exception( + "Replacement argument isn't a standard signed or unsigned integer type", SV("hello {:.{}}"), world, universe); check_exception("Using manual argument numbering in automatic argument numbering mode", SV("hello {:.{0}}"), world, 1); check_exception("Using automatic argument numbering in manual argument numbering mode", SV("hello {0:.{}}"), world, @@ -662,7 +662,7 @@ #ifndef TEST_HAS_NO_INT128 format_test_integer<__int128_t, CharT>(check, check_exception); #endif - // *** check the minma and maxima *** + // *** check the minima and maxima *** check(SV("-0b10000000"), SV("{:#b}"), std::numeric_limits::min()); check(SV("-0200"), SV("{:#o}"), std::numeric_limits::min()); check(SV("-128"), SV("{:#}"), std::numeric_limits::min()); diff --git a/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp b/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.pass.cpp @@ -0,0 +1,90 @@ +//===----------------------------------------------------------------------===// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// UNSUPPORTED: libcpp-has-no-incomplete-format + +// XFAIL: availability-fp_to_chars-missing + +// + +// [format.string.std]/8 +// If { arg-idopt } is used in a width or precision, the value of the +// corresponding formatting argument is used in its place. If the +// corresponding formatting argument is not of standard signed or unsigned +// integer type, or its value is negative for precision or non-positive for +// width, an exception of type format_error is thrown. +// +// This test does the run-time validation + +#include +#include + +#include "assert_macros.h" +#include "concat_macros.h" +#include "format.functions.common.h" +#include "make_string.h" +#include "test_macros.h" + +#define SV(S) MAKE_STRING_VIEW(CharT, S) + +template +void test_exception([[maybe_unused]] std::basic_string_view fmt, [[maybe_unused]] Args&&... args) { + [[maybe_unused]] std::string_view what = "Replacement argument isn't a standard signed or unsigned integer type"; + TEST_VALIDATE_EXCEPTION( + std::format_error, + [&]([[maybe_unused]] const std::format_error& e) { + TEST_LIBCPP_REQUIRE( + e.what() == what, + TEST_WRITE_CONCATENATED( + "\nFormat string ", fmt, "\nExpected exception ", what, "\nActual exception ", e.what(), '\n')); + }, + TEST_IGNORE_NODISCARD std::vformat(fmt, std::make_format_args>(args...))); +} + +template +void test() { + // *** Width *** + test_exception(SV("{:{}}"), 42, true); + test_exception(SV("{:{}}"), 42, '0'); +#ifndef TEST_HAS_NO_WIDE_CHARACTERS + if constexpr (std::same_as) + test_exception(SV("{:{}}"), 42, L'0'); +#endif +#ifndef TEST_HAS_NO_INT128 + test_exception(SV("{:{}}"), 42, __int128_t(0)); + test_exception(SV("{:{}}"), 42, __uint128_t(0)); +#endif + test_exception(SV("{:{}}"), 42, 42.0f); + test_exception(SV("{:{}}"), 42, 42.0); + test_exception(SV("{:{}}"), 42, 42.0l); + + // *** Precision *** + test_exception(SV("{:0.{}}"), 42.0, true); + test_exception(SV("{:0.{}}"), 42.0, '0'); +#ifndef TEST_HAS_NO_WIDE_CHARACTERS + if constexpr (std::same_as) + test_exception(SV("{:0.{}}"), 42.0, L'0'); +#endif +#ifndef TEST_HAS_NO_INT128 + test_exception(SV("{:0.{}}"), 42.0, __int128_t(0)); + test_exception(SV("{:0.{}}"), 42.0, __uint128_t(0)); +#endif + test_exception(SV("{:0.{}}"), 42.0, 42.0f); + test_exception(SV("{:0.{}}"), 42.0, 42.0); + test_exception(SV("{:0.{}}"), 42.0, 42.0l); +} + +int main(int, char**) { + test(); + +#ifndef TEST_HAS_NO_WIDE_CHARACTERS + test(); +#endif + + return 0; +} diff --git a/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.verify.cpp b/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.verify.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/std/utilities/format/format.string/format.string.std/lwg3720_arg_id_width_precision_allowed_types.verify.cpp @@ -0,0 +1,109 @@ +//===----------------------------------------------------------------------===// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// UNSUPPORTED: libcpp-has-no-incomplete-format + +// XFAIL: availability-fp_to_chars-missing + +// + +// [format.string.std]/8 +// If { arg-idopt } is used in a width or precision, the value of the +// corresponding formatting argument is used in its place. If the +// corresponding formatting argument is not of standard signed or unsigned +// integer type, or its value is negative for precision or non-positive for +// width, an exception of type format_error is thrown. +// +// This test does the compile-time validation + +#include + +#include "test_macros.h" + +void test_char_width() { + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, true); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, '0'); +#ifndef TEST_HAS_NO_INT128 + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, __int128_t(0)); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, __uint128_t(0)); +#endif + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, 42.0f); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, 42.0); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:{}}", 42, 42.0l); +} + +void test_char_precision() { + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, true); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, '0'); +#ifndef TEST_HAS_NO_INT128 + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, __int128_t(0)); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, __uint128_t(0)); +#endif + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, 42.0f); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, 42.0); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format("{:0.{}}", 42.0, 42.0l); +} + +#ifndef TEST_HAS_NO_WIDE_CHARACTERS +void test_wchar_t_width() { + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, true); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, '0'); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, L'0'); +# ifndef TEST_HAS_NO_INT128 + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, __int128_t(0)); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, __uint128_t(0)); +# endif + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, 42.0f); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, 42.0); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:{}}", 42, 42.0l); +} + +void test_wchar_t_precision() { + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, true); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, '0'); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, L'0'); +# ifndef TEST_HAS_NO_INT128 + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, __int128_t(0)); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, __uint128_t(0)); +# endif + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, 42.0f); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, 42.0); + // expected-error-re@*:* {{call to consteval function {{.*}} is not a constant expression}} + TEST_IGNORE_NODISCARD std::format(L"{:0.{}}", 42.0, 42.0l); +} + +#endif // TEST_HAS_NO_WIDE_CHARACTERS