Skip to content

Commit 8734a58

Browse files
jviottijviotti
authored
Fix Decimal sign dropping on copy/move (#2033)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent daf4fe5 commit 8734a58

File tree

2 files changed

+161
-5
lines changed

2 files changed

+161
-5
lines changed

‎src/lang/numeric/decimal.cc‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,16 @@ Decimal::~Decimal() {
8282

8383
Decimal::Decimal(const Decimal &other) {
8484
new (this->storage) Data{};
85-
this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA;
85+
this->data()->value.flags =
86+
(other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA;
8687
this->data()->value.exp = 0;
8788
this->data()->value.digits = 0;
8889
this->data()->value.len = 0;
8990
this->data()->value.alloc = Data::STATIC_BUFFER_SIZE;
9091
this->data()->value.data = this->data()->buffer;
9192

92-
if (!mpd_qcopy(&this->data()->value, &other.data()->value,
93-
&decimal_context.status)) {
93+
std::uint32_t status = 0;
94+
if (!mpd_qcopy(&this->data()->value, &other.data()->value, &status)) {
9495
throw NumericOutOfMemoryError{};
9596
}
9697
}
@@ -99,7 +100,8 @@ Decimal::Decimal(Decimal &&other) noexcept {
99100
new (this->storage) Data{};
100101
if (other.data()->value.data == other.data()->buffer) {
101102
// Other uses static storage, copy the data
102-
this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA;
103+
this->data()->value.flags =
104+
(other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA;
103105
this->data()->value.exp = other.data()->value.exp;
104106
this->data()->value.digits = other.data()->value.digits;
105107
this->data()->value.len = other.data()->value.len;
@@ -144,7 +146,8 @@ auto Decimal::operator=(Decimal &&other) noexcept -> Decimal & {
144146

145147
if (other.data()->value.data == other.data()->buffer) {
146148
// Other uses static storage, copy the data
147-
this->data()->value.flags = MPD_STATIC | MPD_STATIC_DATA;
149+
this->data()->value.flags =
150+
(other.data()->value.flags & MPD_NEG) | MPD_STATIC | MPD_STATIC_DATA;
148151
this->data()->value.exp = other.data()->value.exp;
149152
this->data()->value.digits = other.data()->value.digits;
150153
this->data()->value.len = other.data()->value.len;

‎test/numeric/numeric_decimal_test.cc‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,11 @@ TEST(Numeric_decimal, to_string_negative) {
979979
EXPECT_EQ(value.to_string(), "-999");
980980
}
981981

982+
TEST(Numeric_decimal, to_string_negative_decimal) {
983+
const sourcemeta::core::Decimal value{"-67.89"};
984+
EXPECT_EQ(value.to_string(), "-67.89");
985+
}
986+
982987
TEST(Numeric_decimal, to_string_large_number) {
983988
const sourcemeta::core::Decimal value{"123456789012345678901234567890"};
984989
EXPECT_EQ(value.to_string(), "123456789012345678901234567890");
@@ -1142,3 +1147,151 @@ TEST(Numeric_decimal, exception_overflow_addition) {
11421147
{ const auto result = large + addend; },
11431148
sourcemeta::core::NumericOverflowError);
11441149
}
1150+
1151+
TEST(Numeric_decimal, copy_constructor_preserves_negative_sign) {
1152+
const sourcemeta::core::Decimal original{"-123.456"};
1153+
const sourcemeta::core::Decimal copy{original};
1154+
EXPECT_EQ(copy.to_string(), "-123.456");
1155+
EXPECT_TRUE(copy.is_signed());
1156+
}
1157+
1158+
TEST(Numeric_decimal, move_constructor_preserves_negative_sign) {
1159+
sourcemeta::core::Decimal original{"-789.012"};
1160+
const sourcemeta::core::Decimal moved{std::move(original)};
1161+
EXPECT_EQ(moved.to_string(), "-789.012");
1162+
EXPECT_TRUE(moved.is_signed());
1163+
}
1164+
1165+
TEST(Numeric_decimal, copy_assignment_preserves_negative_sign) {
1166+
const sourcemeta::core::Decimal original{"-999.999"};
1167+
sourcemeta::core::Decimal target{42};
1168+
target = original;
1169+
EXPECT_EQ(target.to_string(), "-999.999");
1170+
EXPECT_TRUE(target.is_signed());
1171+
}
1172+
1173+
TEST(Numeric_decimal, move_assignment_preserves_negative_sign) {
1174+
sourcemeta::core::Decimal original{"-111.222"};
1175+
sourcemeta::core::Decimal target{99};
1176+
target = std::move(original);
1177+
EXPECT_EQ(target.to_string(), "-111.222");
1178+
EXPECT_TRUE(target.is_signed());
1179+
}
1180+
1181+
TEST(Numeric_decimal, multiple_copies_preserve_negative_sign) {
1182+
const sourcemeta::core::Decimal first{"-55.55"};
1183+
const sourcemeta::core::Decimal second{first};
1184+
const sourcemeta::core::Decimal third{second};
1185+
const sourcemeta::core::Decimal fourth{third};
1186+
EXPECT_EQ(fourth.to_string(), "-55.55");
1187+
EXPECT_TRUE(fourth.is_signed());
1188+
}
1189+
1190+
TEST(Numeric_decimal, copy_then_move_preserves_negative_sign) {
1191+
const sourcemeta::core::Decimal original{"-333.333"};
1192+
sourcemeta::core::Decimal copy{original};
1193+
const sourcemeta::core::Decimal moved{std::move(copy)};
1194+
EXPECT_EQ(moved.to_string(), "-333.333");
1195+
EXPECT_TRUE(moved.is_signed());
1196+
}
1197+
1198+
TEST(Numeric_decimal, move_then_copy_preserves_negative_sign) {
1199+
sourcemeta::core::Decimal original{"-444.444"};
1200+
const sourcemeta::core::Decimal moved{std::move(original)};
1201+
const sourcemeta::core::Decimal copy{moved};
1202+
EXPECT_EQ(copy.to_string(), "-444.444");
1203+
EXPECT_TRUE(copy.is_signed());
1204+
}
1205+
1206+
TEST(Numeric_decimal, negative_integer_copy) {
1207+
const sourcemeta::core::Decimal original{-12345};
1208+
const sourcemeta::core::Decimal copy{original};
1209+
EXPECT_EQ(copy.to_int64(), -12345);
1210+
EXPECT_TRUE(copy.is_signed());
1211+
}
1212+
1213+
TEST(Numeric_decimal, negative_integer_move) {
1214+
sourcemeta::core::Decimal original{-98765};
1215+
const sourcemeta::core::Decimal moved{std::move(original)};
1216+
EXPECT_EQ(moved.to_int64(), -98765);
1217+
EXPECT_TRUE(moved.is_signed());
1218+
}
1219+
1220+
TEST(Numeric_decimal, very_small_negative_copy) {
1221+
const sourcemeta::core::Decimal original{"-0.000000000000001"};
1222+
const sourcemeta::core::Decimal copy{original};
1223+
EXPECT_TRUE(copy.is_signed());
1224+
EXPECT_TRUE(copy < sourcemeta::core::Decimal{0});
1225+
}
1226+
1227+
TEST(Numeric_decimal, very_small_negative_move) {
1228+
sourcemeta::core::Decimal original{"-0.000000000000001"};
1229+
const sourcemeta::core::Decimal moved{std::move(original)};
1230+
EXPECT_TRUE(moved.is_signed());
1231+
EXPECT_TRUE(moved < sourcemeta::core::Decimal{0});
1232+
}
1233+
1234+
TEST(Numeric_decimal, large_negative_number_copy) {
1235+
const sourcemeta::core::Decimal original{"-999999999999999999999999999999"};
1236+
const sourcemeta::core::Decimal copy{original};
1237+
EXPECT_EQ(copy.to_string(), "-999999999999999999999999999999");
1238+
EXPECT_TRUE(copy.is_signed());
1239+
}
1240+
1241+
TEST(Numeric_decimal, large_negative_number_move) {
1242+
sourcemeta::core::Decimal original{"-888888888888888888888888888888"};
1243+
const sourcemeta::core::Decimal moved{std::move(original)};
1244+
EXPECT_EQ(moved.to_string(), "-888888888888888888888888888888");
1245+
EXPECT_TRUE(moved.is_signed());
1246+
}
1247+
1248+
TEST(Numeric_decimal, negative_scientific_notation_copy) {
1249+
const sourcemeta::core::Decimal original{"-1.23e50"};
1250+
const sourcemeta::core::Decimal copy{original};
1251+
EXPECT_TRUE(copy.is_signed());
1252+
EXPECT_TRUE(copy < sourcemeta::core::Decimal{0});
1253+
}
1254+
1255+
TEST(Numeric_decimal, negative_scientific_notation_move) {
1256+
sourcemeta::core::Decimal original{"-9.87e-20"};
1257+
const sourcemeta::core::Decimal moved{std::move(original)};
1258+
EXPECT_TRUE(moved.is_signed());
1259+
EXPECT_TRUE(moved < sourcemeta::core::Decimal{0});
1260+
}
1261+
1262+
TEST(Numeric_decimal, assignment_chain_with_negative) {
1263+
const sourcemeta::core::Decimal original{"-777.777"};
1264+
sourcemeta::core::Decimal first{1};
1265+
sourcemeta::core::Decimal second{2};
1266+
sourcemeta::core::Decimal third{3};
1267+
first = original;
1268+
second = first;
1269+
third = second;
1270+
EXPECT_EQ(third.to_string(), "-777.777");
1271+
EXPECT_TRUE(third.is_signed());
1272+
}
1273+
1274+
TEST(Numeric_decimal, positive_not_signed_after_copy) {
1275+
const sourcemeta::core::Decimal original{"123.456"};
1276+
const sourcemeta::core::Decimal copy{original};
1277+
EXPECT_FALSE(copy.is_signed());
1278+
}
1279+
1280+
TEST(Numeric_decimal, positive_not_signed_after_move) {
1281+
sourcemeta::core::Decimal original{"789.012"};
1282+
const sourcemeta::core::Decimal moved{std::move(original)};
1283+
EXPECT_FALSE(moved.is_signed());
1284+
}
1285+
1286+
TEST(Numeric_decimal, zero_not_signed_after_copy) {
1287+
const sourcemeta::core::Decimal original{0};
1288+
const sourcemeta::core::Decimal copy{original};
1289+
EXPECT_FALSE(copy.is_signed());
1290+
}
1291+
1292+
TEST(Numeric_decimal, negative_zero_preserves_sign) {
1293+
const sourcemeta::core::Decimal original{"-0"};
1294+
const sourcemeta::core::Decimal copy{original};
1295+
EXPECT_TRUE(copy.is_signed());
1296+
EXPECT_TRUE(copy.is_zero());
1297+
}

0 commit comments

Comments
 (0)