From 6557e73e1d41e49c52a57571c3bedc9ef38fc140 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Tue, 11 Nov 2025 09:45:56 -0400 Subject: [PATCH] Fix `Decimal` sign dropping on copy/move Signed-off-by: Juan Cruz Viotti --- src/lang/numeric/decimal.cc | 13 ++- test/numeric/numeric_decimal_test.cc | 153 +++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 5 deletions(-) diff --git a/src/lang/numeric/decimal.cc b/src/lang/numeric/decimal.cc index b4529df51..8cd41e650 100644 --- a/src/lang/numeric/decimal.cc +++ b/src/lang/numeric/decimal.cc @@ -82,15 +82,16 @@ Decimal::~Decimal() { Decimal::Decimal(const Decimal &other) { new (this->storage) Data{}; - this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA; + this->data()->value.flags = + (other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA; this->data()->value.exp = 0; this->data()->value.digits = 0; this->data()->value.len = 0; this->data()->value.alloc = Data::STATIC_BUFFER_SIZE; this->data()->value.data = this->data()->buffer; - if (!mpd_qcopy(&this->data()->value, &other.data()->value, - &decimal_context.status)) { + std::uint32_t status = 0; + if (!mpd_qcopy(&this->data()->value, &other.data()->value, &status)) { throw NumericOutOfMemoryError{}; } } @@ -99,7 +100,8 @@ Decimal::Decimal(Decimal &&other) noexcept { new (this->storage) Data{}; if (other.data()->value.data == other.data()->buffer) { // Other uses static storage, copy the data - this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA; + this->data()->value.flags = + (other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA; this->data()->value.exp = other.data()->value.exp; this->data()->value.digits = other.data()->value.digits; this->data()->value.len = other.data()->value.len; @@ -144,7 +146,8 @@ auto Decimal::operator=(Decimal &&other) noexcept -> Decimal & { if (other.data()->value.data == other.data()->buffer) { // Other uses static storage, copy the data - this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA; + this->data()->value.flags = + (other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA; this->data()->value.exp = other.data()->value.exp; this->data()->value.digits = other.data()->value.digits; this->data()->value.len = other.data()->value.len; diff --git a/test/numeric/numeric_decimal_test.cc b/test/numeric/numeric_decimal_test.cc index 9b2cc2cb5..b1aa0b67f 100644 --- a/test/numeric/numeric_decimal_test.cc +++ b/test/numeric/numeric_decimal_test.cc @@ -979,6 +979,11 @@ TEST(Numeric_decimal, to_string_negative) { EXPECT_EQ(value.to_string(), "-999"); } +TEST(Numeric_decimal, to_string_negative_decimal) { + const sourcemeta::core::Decimal value{"-67.89"}; + EXPECT_EQ(value.to_string(), "-67.89"); +} + TEST(Numeric_decimal, to_string_large_number) { const sourcemeta::core::Decimal value{"123456789012345678901234567890"}; EXPECT_EQ(value.to_string(), "123456789012345678901234567890"); @@ -1142,3 +1147,151 @@ TEST(Numeric_decimal, exception_overflow_addition) { { const auto result = large + addend; }, sourcemeta::core::NumericOverflowError); } + +TEST(Numeric_decimal, copy_constructor_preserves_negative_sign) { + const sourcemeta::core::Decimal original{"-123.456"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_EQ(copy.to_string(), "-123.456"); + EXPECT_TRUE(copy.is_signed()); +} + +TEST(Numeric_decimal, move_constructor_preserves_negative_sign) { + sourcemeta::core::Decimal original{"-789.012"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_EQ(moved.to_string(), "-789.012"); + EXPECT_TRUE(moved.is_signed()); +} + +TEST(Numeric_decimal, copy_assignment_preserves_negative_sign) { + const sourcemeta::core::Decimal original{"-999.999"}; + sourcemeta::core::Decimal target{42}; + target = original; + EXPECT_EQ(target.to_string(), "-999.999"); + EXPECT_TRUE(target.is_signed()); +} + +TEST(Numeric_decimal, move_assignment_preserves_negative_sign) { + sourcemeta::core::Decimal original{"-111.222"}; + sourcemeta::core::Decimal target{99}; + target = std::move(original); + EXPECT_EQ(target.to_string(), "-111.222"); + EXPECT_TRUE(target.is_signed()); +} + +TEST(Numeric_decimal, multiple_copies_preserve_negative_sign) { + const sourcemeta::core::Decimal first{"-55.55"}; + const sourcemeta::core::Decimal second{first}; + const sourcemeta::core::Decimal third{second}; + const sourcemeta::core::Decimal fourth{third}; + EXPECT_EQ(fourth.to_string(), "-55.55"); + EXPECT_TRUE(fourth.is_signed()); +} + +TEST(Numeric_decimal, copy_then_move_preserves_negative_sign) { + const sourcemeta::core::Decimal original{"-333.333"}; + sourcemeta::core::Decimal copy{original}; + const sourcemeta::core::Decimal moved{std::move(copy)}; + EXPECT_EQ(moved.to_string(), "-333.333"); + EXPECT_TRUE(moved.is_signed()); +} + +TEST(Numeric_decimal, move_then_copy_preserves_negative_sign) { + sourcemeta::core::Decimal original{"-444.444"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + const sourcemeta::core::Decimal copy{moved}; + EXPECT_EQ(copy.to_string(), "-444.444"); + EXPECT_TRUE(copy.is_signed()); +} + +TEST(Numeric_decimal, negative_integer_copy) { + const sourcemeta::core::Decimal original{-12345}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_EQ(copy.to_int64(), -12345); + EXPECT_TRUE(copy.is_signed()); +} + +TEST(Numeric_decimal, negative_integer_move) { + sourcemeta::core::Decimal original{-98765}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_EQ(moved.to_int64(), -98765); + EXPECT_TRUE(moved.is_signed()); +} + +TEST(Numeric_decimal, very_small_negative_copy) { + const sourcemeta::core::Decimal original{"-0.000000000000001"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_TRUE(copy.is_signed()); + EXPECT_TRUE(copy < sourcemeta::core::Decimal{0}); +} + +TEST(Numeric_decimal, very_small_negative_move) { + sourcemeta::core::Decimal original{"-0.000000000000001"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_TRUE(moved.is_signed()); + EXPECT_TRUE(moved < sourcemeta::core::Decimal{0}); +} + +TEST(Numeric_decimal, large_negative_number_copy) { + const sourcemeta::core::Decimal original{"-999999999999999999999999999999"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_EQ(copy.to_string(), "-999999999999999999999999999999"); + EXPECT_TRUE(copy.is_signed()); +} + +TEST(Numeric_decimal, large_negative_number_move) { + sourcemeta::core::Decimal original{"-888888888888888888888888888888"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_EQ(moved.to_string(), "-888888888888888888888888888888"); + EXPECT_TRUE(moved.is_signed()); +} + +TEST(Numeric_decimal, negative_scientific_notation_copy) { + const sourcemeta::core::Decimal original{"-1.23e50"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_TRUE(copy.is_signed()); + EXPECT_TRUE(copy < sourcemeta::core::Decimal{0}); +} + +TEST(Numeric_decimal, negative_scientific_notation_move) { + sourcemeta::core::Decimal original{"-9.87e-20"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_TRUE(moved.is_signed()); + EXPECT_TRUE(moved < sourcemeta::core::Decimal{0}); +} + +TEST(Numeric_decimal, assignment_chain_with_negative) { + const sourcemeta::core::Decimal original{"-777.777"}; + sourcemeta::core::Decimal first{1}; + sourcemeta::core::Decimal second{2}; + sourcemeta::core::Decimal third{3}; + first = original; + second = first; + third = second; + EXPECT_EQ(third.to_string(), "-777.777"); + EXPECT_TRUE(third.is_signed()); +} + +TEST(Numeric_decimal, positive_not_signed_after_copy) { + const sourcemeta::core::Decimal original{"123.456"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_FALSE(copy.is_signed()); +} + +TEST(Numeric_decimal, positive_not_signed_after_move) { + sourcemeta::core::Decimal original{"789.012"}; + const sourcemeta::core::Decimal moved{std::move(original)}; + EXPECT_FALSE(moved.is_signed()); +} + +TEST(Numeric_decimal, zero_not_signed_after_copy) { + const sourcemeta::core::Decimal original{0}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_FALSE(copy.is_signed()); +} + +TEST(Numeric_decimal, negative_zero_preserves_sign) { + const sourcemeta::core::Decimal original{"-0"}; + const sourcemeta::core::Decimal copy{original}; + EXPECT_TRUE(copy.is_signed()); + EXPECT_TRUE(copy.is_zero()); +}