Skip to content

Commit e8cd470

Browse files
jviottijviotti
authored
Better check for Decimal float/double loss of precision (#2035)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 52b80f3 commit e8cd470

File tree

2 files changed

+216
-48
lines changed

2 files changed

+216
-48
lines changed

‎src/lang/numeric/decimal.cc‎

Lines changed: 54 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
#include <sourcemeta/core/numeric_decimal.h>
22
#include <sourcemeta/core/numeric_error.h>
33

4-
#include <cassert> // assert
5-
#include <cmath> // std::isfinite
6-
#include <cstdint> // std::int64_t, std::uint64_t
7-
#include <cstring> // std::strlen
8-
#include <limits> // std::numeric_limits
9-
#include <new> // placement new
10-
#include <string> // std::string, std::stof, std::stod
11-
#include <utility> // std::move
4+
#include <cassert> // assert
5+
#include <cmath> // std::isfinite
6+
#include <cstdint> // std::int64_t, std::uint64_t
7+
#include <cstring> // std::strlen
8+
#include <iomanip> // std::setprecision
9+
#include <limits> // std::numeric_limits
10+
#include <new> // new
11+
#include <sstream> // std::ostringstream
12+
#include <stdexcept> // std::out_of_range
13+
#include <string> // std::string, std::stof, std::stod
14+
#include <type_traits> // std::is_same_v
15+
#include <utility> // std::move
1216

1317
#include <mpdecimal.h> // mpd_*
1418

@@ -57,6 +61,41 @@ thread_local mpd_context_t max_context = []() {
5761
return context;
5862
}();
5963

64+
template <typename FloatingPointType>
65+
auto is_representable_as_floating_point(
66+
const mpd_t *const mpd_value, const sourcemeta::core::Decimal &decimal)
67+
-> bool {
68+
if (mpd_isnan(mpd_value) || mpd_isinfinite(mpd_value)) {
69+
return true;
70+
}
71+
72+
if (!mpd_isfinite(mpd_value)) {
73+
return false;
74+
}
75+
76+
const std::string decimal_string{decimal.to_scientific_string()};
77+
FloatingPointType converted_value;
78+
try {
79+
if constexpr (std::is_same_v<FloatingPointType, float>) {
80+
converted_value = std::stof(decimal_string);
81+
} else if constexpr (std::is_same_v<FloatingPointType, double>) {
82+
converted_value = std::stod(decimal_string);
83+
}
84+
} catch (const std::out_of_range &) {
85+
return false;
86+
}
87+
88+
if (!std::isfinite(converted_value)) {
89+
return false;
90+
}
91+
92+
std::ostringstream oss;
93+
oss << std::setprecision(std::numeric_limits<FloatingPointType>::max_digits10)
94+
<< converted_value;
95+
const sourcemeta::core::Decimal roundtrip{oss.str()};
96+
return decimal == roundtrip;
97+
}
98+
6099
} // namespace
61100

62101
namespace sourcemeta::core {
@@ -294,15 +333,13 @@ auto Decimal::to_uint32() const -> std::uint32_t {
294333
}
295334

296335
auto Decimal::to_float() const -> float {
297-
assert(this->is_float());
298-
const std::string str{this->to_scientific_string()};
299-
return std::stof(str);
336+
const std::string scientific_notation{this->to_scientific_string()};
337+
return std::stof(scientific_notation);
300338
}
301339

302340
auto Decimal::to_double() const -> double {
303-
assert(this->is_double());
304-
const std::string str{this->to_scientific_string()};
305-
return std::stod(str);
341+
const std::string scientific_notation{this->to_scientific_string()};
342+
return std::stod(scientific_notation);
306343
}
307344

308345
auto Decimal::is_zero() const -> bool {
@@ -323,41 +360,12 @@ auto Decimal::is_real() const -> bool {
323360
}
324361

325362
auto Decimal::is_float() const -> bool {
326-
if (mpd_isnan(&this->data()->value) || mpd_isinfinite(&this->data()->value)) {
327-
return true;
328-
}
329-
330-
if (!mpd_isfinite(&this->data()->value)) {
331-
return false;
332-
}
333-
334-
const std::string str{this->to_scientific_string()};
335-
const float float_value{std::stof(str)};
336-
if (!std::isfinite(float_value)) {
337-
return false;
338-
}
339-
340-
const Decimal roundtrip{std::to_string(float_value)};
341-
return *this == roundtrip;
363+
return is_representable_as_floating_point<float>(&this->data()->value, *this);
342364
}
343365

344366
auto Decimal::is_double() const -> bool {
345-
if (mpd_isnan(&this->data()->value) || mpd_isinfinite(&this->data()->value)) {
346-
return true;
347-
}
348-
349-
if (!mpd_isfinite(&this->data()->value)) {
350-
return false;
351-
}
352-
353-
const std::string str{this->to_scientific_string()};
354-
const double double_value{std::stod(str)};
355-
if (!std::isfinite(double_value)) {
356-
return false;
357-
}
358-
359-
const Decimal roundtrip{std::to_string(double_value)};
360-
return *this == roundtrip;
367+
return is_representable_as_floating_point<double>(&this->data()->value,
368+
*this);
361369
}
362370

363371
auto Decimal::is_int32() const -> bool {

‎test/numeric/numeric_decimal_test.cc‎

Lines changed: 162 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ TEST(Numeric_decimal, is_float_simple_values) {
497497
const sourcemeta::core::Decimal value2{"3.14"};
498498
const sourcemeta::core::Decimal value3{"-2.5"};
499499
EXPECT_TRUE(value1.is_float());
500-
EXPECT_TRUE(value2.is_float());
500+
EXPECT_FALSE(value2.is_float());
501501
EXPECT_TRUE(value3.is_float());
502502
}
503503

@@ -515,12 +515,92 @@ TEST(Numeric_decimal, is_float_high_precision_loss) {
515515
EXPECT_FALSE(value.is_float());
516516
}
517517

518+
TEST(Numeric_decimal, is_float_large_exponent_in_range) {
519+
const sourcemeta::core::Decimal value{"1.5e30"};
520+
EXPECT_FALSE(value.is_float());
521+
}
522+
523+
TEST(Numeric_decimal, is_float_large_negative_exponent_in_range) {
524+
const sourcemeta::core::Decimal value{"1.5e-30"};
525+
EXPECT_TRUE(value.is_float());
526+
}
527+
528+
TEST(Numeric_decimal, is_float_too_large) {
529+
const sourcemeta::core::Decimal value{"1e100"};
530+
EXPECT_FALSE(value.is_float());
531+
}
532+
533+
TEST(Numeric_decimal, is_float_too_small) {
534+
const sourcemeta::core::Decimal value{"1e-100"};
535+
EXPECT_FALSE(value.is_float());
536+
}
537+
538+
TEST(Numeric_decimal, is_float_near_max_float) {
539+
const sourcemeta::core::Decimal value{"3.4e38"};
540+
EXPECT_FALSE(value.is_float());
541+
}
542+
543+
TEST(Numeric_decimal, is_float_exceeds_max_float) {
544+
const sourcemeta::core::Decimal value{"4e38"};
545+
EXPECT_FALSE(value.is_float());
546+
}
547+
548+
TEST(Numeric_decimal, is_float_near_min_float) {
549+
const sourcemeta::core::Decimal value{"1.2e-38"};
550+
EXPECT_FALSE(value.is_float());
551+
}
552+
553+
TEST(Numeric_decimal, is_float_below_min_float) {
554+
const sourcemeta::core::Decimal value{"1e-50"};
555+
EXPECT_FALSE(value.is_float());
556+
}
557+
558+
TEST(Numeric_decimal, is_float_6_significant_digits) {
559+
const sourcemeta::core::Decimal value{"1.234375"};
560+
EXPECT_TRUE(value.is_float());
561+
}
562+
563+
TEST(Numeric_decimal, is_float_7_significant_digits_no_loss) {
564+
const sourcemeta::core::Decimal value{"1.2343750"};
565+
EXPECT_TRUE(value.is_float());
566+
}
567+
568+
TEST(Numeric_decimal, is_float_many_digits_with_loss) {
569+
const sourcemeta::core::Decimal value{"1.23456789"};
570+
EXPECT_FALSE(value.is_float());
571+
}
572+
573+
TEST(Numeric_decimal, is_float_big_integer_in_float_range) {
574+
const sourcemeta::core::Decimal value{"16777216"};
575+
EXPECT_TRUE(value.is_float());
576+
}
577+
578+
TEST(Numeric_decimal, is_float_big_integer_exceeds_precision) {
579+
const sourcemeta::core::Decimal value{"16777217"};
580+
EXPECT_FALSE(value.is_float());
581+
}
582+
583+
TEST(Numeric_decimal, is_float_small_value_with_exponent) {
584+
const sourcemeta::core::Decimal value{"1.0e-10"};
585+
EXPECT_FALSE(value.is_float());
586+
}
587+
588+
TEST(Numeric_decimal, is_float_zero) {
589+
const sourcemeta::core::Decimal value{"0.0"};
590+
EXPECT_TRUE(value.is_float());
591+
}
592+
593+
TEST(Numeric_decimal, is_float_negative_zero) {
594+
const sourcemeta::core::Decimal value{"-0.0"};
595+
EXPECT_TRUE(value.is_float());
596+
}
597+
518598
TEST(Numeric_decimal, is_double_simple_values) {
519599
const sourcemeta::core::Decimal value1{3};
520600
const sourcemeta::core::Decimal value2{"3.14"};
521601
const sourcemeta::core::Decimal value3{"-2.5"};
522602
EXPECT_TRUE(value1.is_double());
523-
EXPECT_TRUE(value2.is_double());
603+
EXPECT_FALSE(value2.is_double());
524604
EXPECT_TRUE(value3.is_double());
525605
}
526606

@@ -539,6 +619,86 @@ TEST(Numeric_decimal, is_double_high_precision_loss) {
539619
EXPECT_FALSE(value.is_double());
540620
}
541621

622+
TEST(Numeric_decimal, is_double_large_exponent_in_range) {
623+
const sourcemeta::core::Decimal value{"1.5e100"};
624+
EXPECT_FALSE(value.is_double());
625+
}
626+
627+
TEST(Numeric_decimal, is_double_large_negative_exponent_in_range) {
628+
const sourcemeta::core::Decimal value{"1.5e-100"};
629+
EXPECT_TRUE(value.is_double());
630+
}
631+
632+
TEST(Numeric_decimal, is_double_too_large) {
633+
const sourcemeta::core::Decimal value{"1e500"};
634+
EXPECT_FALSE(value.is_double());
635+
}
636+
637+
TEST(Numeric_decimal, is_double_too_small) {
638+
const sourcemeta::core::Decimal value{"1e-500"};
639+
EXPECT_FALSE(value.is_double());
640+
}
641+
642+
TEST(Numeric_decimal, is_double_near_max_double) {
643+
const sourcemeta::core::Decimal value{"1.7e308"};
644+
EXPECT_FALSE(value.is_double());
645+
}
646+
647+
TEST(Numeric_decimal, is_double_exceeds_max_double) {
648+
const sourcemeta::core::Decimal value{"2e308"};
649+
EXPECT_FALSE(value.is_double());
650+
}
651+
652+
TEST(Numeric_decimal, is_double_near_min_double) {
653+
const sourcemeta::core::Decimal value{"2.2e-308"};
654+
EXPECT_FALSE(value.is_double());
655+
}
656+
657+
TEST(Numeric_decimal, is_double_below_min_double) {
658+
const sourcemeta::core::Decimal value{"1e-400"};
659+
EXPECT_FALSE(value.is_double());
660+
}
661+
662+
TEST(Numeric_decimal, is_double_15_significant_digits) {
663+
const sourcemeta::core::Decimal value{"1.23456789012345"};
664+
EXPECT_TRUE(value.is_double());
665+
}
666+
667+
TEST(Numeric_decimal, is_double_16_significant_digits_no_loss) {
668+
const sourcemeta::core::Decimal value{"1.234567890123456"};
669+
EXPECT_TRUE(value.is_double());
670+
}
671+
672+
TEST(Numeric_decimal, is_double_many_digits_with_loss) {
673+
const sourcemeta::core::Decimal value{"1.234567890123456789"};
674+
EXPECT_FALSE(value.is_double());
675+
}
676+
677+
TEST(Numeric_decimal, is_double_big_integer_in_double_range) {
678+
const sourcemeta::core::Decimal value{"9007199254740992"};
679+
EXPECT_TRUE(value.is_double());
680+
}
681+
682+
TEST(Numeric_decimal, is_double_big_integer_exceeds_precision) {
683+
const sourcemeta::core::Decimal value{"9007199254740993"};
684+
EXPECT_FALSE(value.is_double());
685+
}
686+
687+
TEST(Numeric_decimal, is_double_small_value_with_exponent) {
688+
const sourcemeta::core::Decimal value{"1.0e-28"};
689+
EXPECT_FALSE(value.is_double());
690+
}
691+
692+
TEST(Numeric_decimal, is_double_zero) {
693+
const sourcemeta::core::Decimal value{"0.0"};
694+
EXPECT_TRUE(value.is_double());
695+
}
696+
697+
TEST(Numeric_decimal, is_double_negative_zero) {
698+
const sourcemeta::core::Decimal value{"-0.0"};
699+
EXPECT_TRUE(value.is_double());
700+
}
701+
542702
TEST(Numeric_decimal, to_float_simple) {
543703
const sourcemeta::core::Decimal value{"3.14"};
544704
EXPECT_FLOAT_EQ(value.to_float(), 3.14f);

0 commit comments

Comments
 (0)