[libcxx-commits] [libcxx] f92c733 - [libc++][format] Fixes floating-point formatting.
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Aug 31 10:26:03 PDT 2022
Author: Mark de Wever
Date: 2022-08-31T19:25:53+02:00
New Revision: f92c733cc26d40fada0e89005d38d3f6f0bf413a
URL: https://github.com/llvm/llvm-project/commit/f92c733cc26d40fada0e89005d38d3f6f0bf413a
DIFF: https://github.com/llvm/llvm-project/commit/f92c733cc26d40fada0e89005d38d3f6f0bf413a.diff
LOG: [libc++][format] Fixes floating-point formatting.
Formatting the alternate form for the general categories should keep the
trailing zeros. This was reported by @fsb4000 in D131336.
The default format uses general formatting but this should not keep the
trailing zeros so the default format is not passed to the formatter.
While testing I found an off by one error; finding the exponent character
`e` in 1e+03 will start at after the `1` so a size of `4` can contain an
exponent.
Reviewed By: fsb4000, ldionne, #libc
Differential Revision: https://reviews.llvm.org/D131417
Added:
Modified:
libcxx/include/__format/formatter_floating_point.h
libcxx/include/__format/parser_std_format_spec.h
libcxx/test/std/utilities/format/format.functions/format_tests.h
Removed:
################################################################################
diff --git a/libcxx/include/__format/formatter_floating_point.h b/libcxx/include/__format/formatter_floating_point.h
index 097dbe66ceb9..29b3711ef9eb 100644
--- a/libcxx/include/__format/formatter_floating_point.h
+++ b/libcxx/include/__format/formatter_floating_point.h
@@ -12,6 +12,7 @@
#include <__algorithm/copy_n.h>
#include <__algorithm/find.h>
+#include <__algorithm/max.h>
#include <__algorithm/min.h>
#include <__algorithm/rotate.h>
#include <__algorithm/transform.h>
@@ -181,6 +182,7 @@ class _LIBCPP_TEMPLATE_VIS __float_buffer {
_LIBCPP_HIDE_FROM_ABI int __precision() const { return __precision_; }
_LIBCPP_HIDE_FROM_ABI int __num_trailing_zeros() const { return __num_trailing_zeros_; }
_LIBCPP_HIDE_FROM_ABI void __remove_trailing_zeros() { __num_trailing_zeros_ = 0; }
+ _LIBCPP_HIDE_FROM_ABI void __add_trailing_zeros(int __zeros) { __num_trailing_zeros_ += __zeros; }
private:
int __precision_;
@@ -214,7 +216,7 @@ struct __float_result {
/// \returns a pointer to the exponent or __last when not found.
constexpr inline _LIBCPP_HIDE_FROM_ABI char* __find_exponent(char* __first, char* __last) {
ptr
diff _t __size = __last - __first;
- if (__size > 4) {
+ if (__size >= 4) {
__first = __last - _VSTD::min(__size, ptr
diff _t(6));
for (; __first != __last - 3; ++__first) {
if (*__first == 'e')
@@ -402,6 +404,7 @@ _LIBCPP_HIDE_FROM_ABI __float_result __format_buffer_general_lower_case(__float_
// In fixed mode the algorithm truncates trailing spaces and possibly the
// radix point. There's no good guess for the position of the radix point
// therefore scan the output after the first digit.
+
__result.__radix_point = _VSTD::find(__first, __result.__last, '.');
}
}
@@ -452,7 +455,10 @@ _LIBCPP_HIDE_FROM_ABI __float_result __format_buffer(
char* __first = __formatter::__insert_sign(__buffer.begin(), __negative, __sign);
switch (__type) {
case __format_spec::__type::__default:
- return __formatter::__format_buffer_default(__buffer, __value, __first);
+ if (__has_precision)
+ return __formatter::__format_buffer_general_lower_case(__buffer, __value, __buffer.__precision(), __first);
+ else
+ return __formatter::__format_buffer_default(__buffer, __value, __first);
case __format_spec::__type::__hexfloat_lower_case:
return __formatter::__format_buffer_hexadecimal_lower_case(
@@ -623,20 +629,51 @@ __format_floating_point(_Tp __value, auto& __ctx, __format_spec::__parsed_specif
__float_result __result = __formatter::__format_buffer(
__buffer, __value, __negative, (__specs.__has_precision()), __specs.__std_.__sign_, __specs.__std_.__type_);
- if (__specs.__std_.__alternate_form_ && __result.__radix_point == __result.__last) {
- *__result.__last++ = '.';
-
- // When there is an exponent the point needs to be moved before the
- // exponent. When there's no exponent the rotate does nothing. Since
- // rotate tests whether the operation is a nop, call it unconditionally.
- _VSTD::rotate(__result.__exponent, __result.__last - 1, __result.__last);
- __result.__radix_point = __result.__exponent;
+ if (__specs.__std_.__alternate_form_) {
+ if (__result.__radix_point == __result.__last) {
+ *__result.__last++ = '.';
+
+ // When there is an exponent the point needs to be moved before the
+ // exponent. When there's no exponent the rotate does nothing. Since
+ // rotate tests whether the operation is a nop, call it unconditionally.
+ _VSTD::rotate(__result.__exponent, __result.__last - 1, __result.__last);
+ __result.__radix_point = __result.__exponent;
+
+ // The radix point is always placed before the exponent.
+ // - No exponent needs to point to the new last.
+ // - An exponent needs to move one position to the right.
+ // So it's safe to increment the value unconditionally.
+ ++__result.__exponent;
+ }
- // The radix point is always placed before the exponent.
- // - No exponent needs to point to the new last.
- // - An exponent needs to move one position to the right.
- // So it's safe to increment the value unconditionally.
- ++__result.__exponent;
+ // [format.string.std]/6
+ // In addition, for g and G conversions, trailing zeros are not removed
+ // from the result.
+ //
+ // If the type option for a floating-point type is none it may use the
+ // general formatting, but it's not a g or G conversion. So in that case
+ // the formatting should not append trailing zeros.
+ bool __is_general = __specs.__std_.__type_ == __format_spec::__type::__general_lower_case ||
+ __specs.__std_.__type_ == __format_spec::__type::__general_upper_case;
+
+ if (__is_general) {
+ // https://en.cppreference.com/w/c/io/fprintf
+ // Let P equal the precision if nonzero, 6 if the precision is not
+ // specified, or 1 if the precision is 0. Then, if a conversion with
+ // style E would have an exponent of X:
+ int __p = _VSTD::max(1, (__specs.__has_precision() ? __specs.__precision_ : 6));
+ if (__result.__exponent == __result.__last)
+ // if P > X >= -4, the conversion is with style f or F and precision P - 1 - X.
+ // By including the radix point it calculates P - (1 + X)
+ __p -= __result.__radix_point - __buffer.begin();
+ else
+ // otherwise, the conversion is with style e or E and precision P - 1.
+ --__p;
+
+ ptr
diff _t __precision = (__result.__exponent - __result.__radix_point) - 1;
+ if (__precision < __p)
+ __buffer.__add_trailing_zeros(__p - __precision);
+ }
}
# ifndef _LIBCPP_HAS_NO_LOCALIZATION
diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h
index 5dfcb5fb7e5f..576cded812bb 100644
--- a/libcxx/include/__format/parser_std_format_spec.h
+++ b/libcxx/include/__format/parser_std_format_spec.h
@@ -650,11 +650,6 @@ template <class _CharT>
_LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_floating_point(__parser<_CharT>& __parser) {
switch (__parser.__type_) {
case __format_spec::__type::__default:
- // When no precision specified then it keeps default since that
- // formatting
diff ers from the other types.
- if (__parser.__precision_as_arg_ || __parser.__precision_ != -1)
- __parser.__type_ = __format_spec::__type::__general_lower_case;
- break;
case __format_spec::__type::__hexfloat_lower_case:
case __format_spec::__type::__hexfloat_upper_case:
// Precision specific behavior will be handled later.
diff --git a/libcxx/test/std/utilities/format/format.functions/format_tests.h b/libcxx/test/std/utilities/format/format.functions/format_tests.h
index 5b5db6403886..641542b749e2 100644
--- a/libcxx/test/std/utilities/format/format.functions/format_tests.h
+++ b/libcxx/test/std/utilities/format/format.functions/format_tests.h
@@ -2037,8 +2037,8 @@ void format_test_floating_point_general_lower_case(TestFunction check) {
check.template operator()<"answer is '{:.0g}'">(SV("answer is '0'"), F(0));
check.template operator()<"answer is '{:#.0g}'">(SV("answer is '0.'"), F(0));
- check.template operator()<"answer is '{:#g}'">(SV("answer is '0.'"), F(0));
- check.template operator()<"answer is '{:#g}'">(SV("answer is '2.5'"), F(2.5));
+ check.template operator()<"answer is '{:#g}'">(SV("answer is '0.00000'"), F(0));
+ check.template operator()<"answer is '{:#g}'">(SV("answer is '2.50000'"), F(2.5));
check.template operator()<"answer is '{:#g}'">(SV("answer is 'inf'"), std::numeric_limits<F>::infinity());
check.template operator()<"answer is '{:#g}'">(SV("answer is '-inf'"), -std::numeric_limits<F>::infinity());
@@ -2086,6 +2086,33 @@ void format_test_floating_point_general_lower_case(TestFunction check) {
check.template operator()<"answer is '{:.5g}'">(SV("answer is '0.03125'"), 0.03125);
check.template operator()<"answer is '{:.10g}'">(SV("answer is '0.03125'"), 0.03125);
+ // *** precision & alternate form ***
+
+ // Output validated with printf("%#xg")
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.'"), 1.2, 0);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.'"), 1.2, 1);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.2'"), 1.2, 2);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.20'"), 1.2, 3);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.200'"), 1.2, 4);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.2000'"), 1.2, 5);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.20000'"), 1.2, 6);
+
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.e+03'"), 1200.0, 0);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.e+03'"), 1200.0, 1);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.2e+03'"), 1200.0, 2);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.20e+03'"), 1200.0, 3);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1200.'"), 1200.0, 4);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1200.0'"), 1200.0, 5);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1200.00'"), 1200.0, 6);
+
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.e+06'"), 1200000.0, 0);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.e+06'"), 1200000.0, 1);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.2e+06'"), 1200000.0, 2);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.20e+06'"), 1200000.0, 3);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.200e+06'"), 1200000.0, 4);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.2000e+06'"), 1200000.0, 5);
+ check.template operator()<"answer is '{:#.{}g}'">(SV("answer is '1.20000e+06'"), 1200000.0, 6);
+
// *** locale-specific form ***
// See locale-specific_form.pass.cpp
}
@@ -2163,8 +2190,8 @@ void format_test_floating_point_general_upper_case(TestFunction check) {
check.template operator()<"answer is '{:.0G}'">(SV("answer is '0'"), F(0));
check.template operator()<"answer is '{:#.0G}'">(SV("answer is '0.'"), F(0));
- check.template operator()<"answer is '{:#G}'">(SV("answer is '0.'"), F(0));
- check.template operator()<"answer is '{:#G}'">(SV("answer is '2.5'"), F(2.5));
+ check.template operator()<"answer is '{:#G}'">(SV("answer is '0.00000'"), F(0));
+ check.template operator()<"answer is '{:#G}'">(SV("answer is '2.50000'"), F(2.5));
check.template operator()<"answer is '{:#G}'">(SV("answer is 'INF'"), std::numeric_limits<F>::infinity());
check.template operator()<"answer is '{:#G}'">(SV("answer is '-INF'"), -std::numeric_limits<F>::infinity());
@@ -2212,6 +2239,33 @@ void format_test_floating_point_general_upper_case(TestFunction check) {
check.template operator()<"answer is '{:.5G}'">(SV("answer is '0.03125'"), 0.03125);
check.template operator()<"answer is '{:.10G}'">(SV("answer is '0.03125'"), 0.03125);
+ // *** precision & alternate form ***
+
+ // Output validated with printf("%#xg")
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.'"), 1.2, 0);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.'"), 1.2, 1);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.2'"), 1.2, 2);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.20'"), 1.2, 3);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.200'"), 1.2, 4);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.2000'"), 1.2, 5);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.20000'"), 1.2, 6);
+
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.E+03'"), 1200.0, 0);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.E+03'"), 1200.0, 1);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.2E+03'"), 1200.0, 2);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.20E+03'"), 1200.0, 3);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1200.'"), 1200.0, 4);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1200.0'"), 1200.0, 5);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1200.00'"), 1200.0, 6);
+
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.E+06'"), 1200000.0, 0);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.E+06'"), 1200000.0, 1);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.2E+06'"), 1200000.0, 2);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.20E+06'"), 1200000.0, 3);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.200E+06'"), 1200000.0, 4);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.2000E+06'"), 1200000.0, 5);
+ check.template operator()<"answer is '{:#.{}G}'">(SV("answer is '1.20000E+06'"), 1200000.0, 6);
+
// *** locale-specific form ***
// See locale-specific_form.pass.cpp
}
@@ -2455,6 +2509,32 @@ void format_test_floating_point_default_precision(TestFunction check) {
check.template operator()<"answer is '{:.5}'">(SV("answer is '0.03125'"), 0.03125);
check.template operator()<"answer is '{:.10}'">(SV("answer is '0.03125'"), 0.03125);
+ // *** precision & alternate form ***
+
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.'"), 1.2, 0);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.'"), 1.2, 1);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2'"), 1.2, 2);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2'"), 1.2, 3);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2'"), 1.2, 4);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2'"), 1.2, 5);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2'"), 1.2, 6);
+
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.e+03'"), 1200.0, 0);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.e+03'"), 1200.0, 1);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+03'"), 1200.0, 2);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+03'"), 1200.0, 3);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1200.'"), 1200.0, 4);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1200.'"), 1200.0, 5);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1200.'"), 1200.0, 6);
+
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.e+06'"), 1200000.0, 0);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.e+06'"), 1200000.0, 1);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+06'"), 1200000.0, 2);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+06'"), 1200000.0, 3);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+06'"), 1200000.0, 4);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+06'"), 1200000.0, 5);
+ check.template operator()<"answer is '{:#.{}}'">(SV("answer is '1.2e+06'"), 1200000.0, 6);
+
// *** locale-specific form ***
// See locale-specific_form.pass.cpp
}
More information about the libcxx-commits
mailing list