Failed TestSwap() and TestFilename() on i386 platform: non-robust floating-point value comparison

[hosiet]

Please read the marisa-trie library build log for i386 platform for 4ef33cc at https://buildd.debian.org/status/fetch.php?pkg=marisa&arch=i386&ver=0.3.1%2Bgit20250817-1%7Eexp1&stamp=1758502849&raw=0 :

1/5 Test #1: base-test ........................***Failed    0.02 sec
./tests/base-test.cc:21: TestSwap(): 30: Assertion `a == 2.34' failed.

2/5 Test #2: io-test ..........................***Failed    0.02 sec
./tests/io-test.cc:22: TestFilename(): 49: Assertion `values[0] == 3.45' failed.

3/5 Test #4: trie-test ........................   Passed    0.01 sec
3: ./tests/vector-test.cc:36: TestPopcount(): ok
3: ./tests/vector-test.cc:47: TestRankIndex(): ok
3: ./tests/vector-test.cc:82: TestVector(): ok
3: ./tests/vector-test.cc:202: TestFlatVector(): ok
3: ./tests/vector-test.cc:386: TestBitVector(): ok
4/5 Test #3: vector-test ......................   Passed    0.06 sec
5: ./tests/marisa-test.cc:22: TestEmptyTrie(): ok
5: ./tests/marisa-test.cc:96: TestTinyTrie(): ok
5: ./tests/marisa-test.cc:445: TestTrie(): TEXT, WEIGHT: ok
5: ./tests/marisa-test.cc:445: TestTrie(): TEXT, LABEL: ok
5: ./tests/marisa-test.cc:445: TestTrie(): BINARY, WEIGHT: ok
5: ./tests/marisa-test.cc:445: TestTrie(): BINARY, LABEL: ok
5/5 Test #5: marisa-test ......................   Passed    1.83 sec

60% tests passed, 2 tests failed out of 5

Total Test time (real) =   1.85 sec

The following tests FAILED:
	  1 - base-test (Failed)
	  2 - io-test (Failed)

When we look at the test code:

marisa-trie/tests/base-test.cc

Lines 28 to 31 in 4ef33cc

	double a = 1.23, b = 2.34;
	std::swap(a, b);
	ASSERT(a == 2.34);
	ASSERT(b == 1.23);

This is not robust since code built on i386 may or may not use x87 floating instruction set. since the literal floating number has excessive precision support enabled by default in the strict standard mode after gcc 13, which means 1.23 == (long double) 1.23 != (double) 1.23. I am not sure if it is intended or it's a GCC bug. The behavior is intentional, see the first line of https://gcc.gnu.org/gcc-13/changes.html#cxx . See the minimal example below:

root@debian-i386:~/tmp# g++ dp.cpp -o ./out_none && ./out_none
a == 1.23: true
b == 2.34: true
root@debian-i386:~/tmp# g++ -std=c++20 dp.cpp -o ./out_none && ./out_none
a == 1.23: false
b == 2.34: false
root@debian-i386:~/tmp# cat dp.cpp
#include <iostream>

int main() {
    double a = 1.23;
    double b = 2.34;

    std::cout << "a == 1.23: " << (a == 1.23 ? "true" : "false") << std::endl;
    std::cout << "b == 2.34: " << (b == 2.34 ? "true" : "false") << std::endl;

    return 0;
}
root@debian-i386:~/src# g++ --version
g++ (Debian 15.2.0-4) 15.2.0
Copyright (C) 2025 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

root@debian-i386:~/src# uname -m
i686

We should not rely on compiler's implementation detail to do tests. I am providing 2 choices to solve this issue:

(1) See the following patch:

diff --git a/tests/base-test.cc b/tests/base-test.cc
index 2f1cfe8..4ec0990 100644
--- a/tests/base-test.cc
+++ b/tests/base-test.cc
@@ -27,8 +27,8 @@ void TestSwap() {

   double a = 1.23, b = 2.34;
   std::swap(a, b);
-  ASSERT(a == 2.34);
-  ASSERT(b == 1.23);
+  ASSERT(a == double(2.34));
+  ASSERT(b == double(1.23));

   TEST_END();
 }
diff --git a/tests/io-test.cc b/tests/io-test.cc
index 668df5a..32c5fb3 100644
--- a/tests/io-test.cc
+++ b/tests/io-test.cc
@@ -28,7 +28,7 @@ void TestFilename() {
     writer.write(std::uint32_t{123});
     writer.write(std::uint32_t{234});

-    double values[] = {3.45, 4.56};
+    double values[] = {double(3.45), double(4.56)};
     writer.write(values, 2);

     EXCEPT(writer.write(values, SIZE_MAX), std::invalid_argument);
@@ -46,8 +46,8 @@ void TestFilename() {

     double values[2];
     reader.read(values, 2);
-    ASSERT(values[0] == 3.45);
-    ASSERT(values[1] == 4.56);
+    ASSERT(values[0] == double(3.45));
+    ASSERT(values[1] == double(4.56));

     char byte;
     EXCEPT(reader.read(&byte), std::runtime_error);
@@ -65,8 +65,8 @@ void TestFilename() {

     const double *values;
     mapper.map(&values, 2);
-    ASSERT(values[0] == 3.45);
-    ASSERT(values[1] == 4.56);
+    ASSERT(values[0] == double(3.45));
+    ASSERT(values[1] == double(4.56));

     char byte;
     EXCEPT(mapper.map(&byte), std::runtime_error);
@@ -84,8 +84,8 @@ void TestFilename() {

     const double *values;
     mapper.map(&values, 2);
-    ASSERT(values[0] == 3.45);
-    ASSERT(values[1] == 4.56);
+    ASSERT(values[0] == double(3.45));
+    ASSERT(values[1] == double(4.56));

     char byte;
     EXCEPT(mapper.map(&byte), std::runtime_error);
@@ -126,7 +126,7 @@ void TestFd() {
     std::uint32_t value = 234;
     writer.write(value);

-    double values[] = {34.5, 67.8};
+    double values[] = {double(34.5), double(67.8)};
     writer.write(values, 2);

 #ifdef _MSC_VER
@@ -154,8 +154,8 @@ void TestFd() {

     double values[2];
     reader.read(values, 2);
-    ASSERT(values[0] == 34.5);
-    ASSERT(values[1] == 67.8);
+    ASSERT(values[0] == double(34.5));
+    ASSERT(values[1] == double(67.8));

     char byte;
     EXCEPT(reader.read(&byte), std::runtime_error);
@@ -187,7 +187,7 @@ void TestFile() {
     std::uint32_t value = 10;
     writer.write(value);

-    double values[2] = {0.1, 0.2};
+    double values[2] = {double(0.1), double(0.2)};
     writer.write(values, 2);

     ASSERT(std::fclose(file) == 0);
@@ -210,8 +210,8 @@ void TestFile() {

     double values[2];
     reader.read(values, 2);
-    ASSERT(values[0] == 0.1);
-    ASSERT(values[1] == 0.2);
+    ASSERT(values[0] == double(0.1));
+    ASSERT(values[1] == double(0.2));

     char byte;
     EXCEPT(reader.read(&byte), std::runtime_error);
@@ -234,7 +234,7 @@ void TestStream() {
     std::uint32_t value = 12;
     writer.write(value);

-    double values[2] = {3.4, 5.6};
+    double values[2] = {double(3.4), double(5.6)};
     writer.write(values, 2);
   }

@@ -248,8 +248,8 @@ void TestStream() {

     double values[2];
     reader.read(values, 2);
-    ASSERT(values[0] == 3.4);
-    ASSERT(values[1] == 5.6);
+    ASSERT(values[0] == double(3.4));
+    ASSERT(values[1] == double(5.6));

     char byte;
     EXCEPT(reader.read(&byte), std::runtime_error);

(2) Replace occurrences of ASSERT(a == b) with ASSERT_DOUBLE_EQ(a, b), as well as #define ASSERT_DOUBLE_EQ(actual, expected) ASSERT(std::abs((actual) - (expected)) < 1e-14).

[jmr]

TestSwap can be deleted. We don't need to test std::swap; it is just leftover from when there was a marisa::swap.

marisa-trie/tests/base-test.cc

Lines 20 to 34 in 4ef33cc

	void TestSwap() {
	TEST_START();
	int x = 100, y = 200;
	std::swap(x, y);
	ASSERT(x == 200);
	ASSERT(y == 100);
	double a = 1.23, b = 2.34;
	std::swap(a, b);
	ASSERT(a == 2.34);
	ASSERT(b == 1.23);
	TEST_END();
	}

For TestFilename, ASSERT_DOUBLE_EQ looks better than all those double() casts. Another option would be just to wrap the test in if constexpr (std::numeric_limits<double>::is_iec559()).