]> granicus.if.org Git - icu/commitdiff
ICU-20080 Avoid strange compiler behaviour in ASSERT_EQUAL() macro.
authorFredrik Roubert <roubert@google.com>
Fri, 12 Oct 2018 12:33:03 +0000 (14:33 +0200)
committerFredrik Roubert <fredrik@roubert.name>
Fri, 19 Oct 2018 09:45:09 +0000 (11:45 +0200)
Using temporary variables for the two values to be compared here makes
GCC compile the code just like we expect it to. (What it really is that
it otherwise does on some architechtures remains a mystery.)

This will make the tests pass as expected also on IA-32 with GCC.

It'll also make it possible to revert the old workaround for SPARC
introduced by commit 5b0592af79c7601d08cafcbbc7b71077faeb7e4f.

Tested:

Linux gcc45 3.16.0-5-686-pae #1 SMP Debian 3.16.51-3+deb8u1 (2018-01-08) i686 GNU/Linux

Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0)
g++ (Debian 4.9.2-10+deb8u1) 4.9.2

Linux gcc202 4.16.0-1-sparc64-smp #1 SMP Debian 4.16.5-1 (2018-04-29) sparc64 GNU/Linux

clang version 4.0.1-10+sparc64 (tags/RELEASE_401/final)
g++ (Debian 8.2.0-7) 8.2.0

icu4c/source/test/intltest/dcfmapts.cpp

index 6a79bab8509c45a1f939c8af5c5761d63082637c..9fa0e3deee4460e0f9a02b4bdca28014d14a8c17 100644 (file)
@@ -636,8 +636,14 @@ void IntlTestDecimalFormatAPI::TestScale()
 }
 
 
-#define ASSERT_EQUAL(expect, actual) { char tmp[200]; sprintf(tmp, "(%g==%g)", (double)(expect), (double)(actual)); \
-    assertTrue(tmp, ((expect)==(actual)), FALSE, TRUE, __FILE__, __LINE__); }
+#define ASSERT_EQUAL(expect, actual) { \
+    /* ICU-20080: Use temporary variables to avoid strange compiler behaviour \
+       (with the nice side-effect of avoiding repeated function calls too). */ \
+    auto lhs = (expect); \
+    auto rhs = (actual); \
+    char tmp[200]; \
+    sprintf(tmp, "(%g==%g)", (double)lhs, (double)rhs); \
+    assertTrue(tmp, (lhs==rhs), FALSE, TRUE, __FILE__, __LINE__); }
 
 void IntlTestDecimalFormatAPI::TestFixedDecimal() {
     UErrorCode status = U_ZERO_ERROR;
@@ -946,16 +952,7 @@ void IntlTestDecimalFormatAPI::TestFixedDecimal() {
     ASSERT_EQUAL(0, fd.getPluralOperand(PLURAL_OPERAND_T));
     // note: going through DigitList path to FixedDecimal, which is trimming
     //       int64_t fields to 18 digits. See ticket Ticket #10374
-    // ASSERT_EQUAL(223372036854775807LL, fd.getPluralOperand(PLURAL_OPERAND_I);
-    if (!(
-            fd.getPluralOperand(PLURAL_OPERAND_I) == 223372036854775807LL ||
-            fd.getPluralOperand(PLURAL_OPERAND_I) == 9223372036854775807LL)) {
-        dataerrln(
-                "File %s, Line %d, fd.getPluralOperand(PLURAL_OPERAND_I = %lld",
-                __FILE__,
-                __LINE__,
-                fd.getPluralOperand(PLURAL_OPERAND_I));
-    }
+    ASSERT_EQUAL(223372036854775807LL, fd.getPluralOperand(PLURAL_OPERAND_I));
     ASSERT_EQUAL(TRUE, fd.hasIntegerValue());
     ASSERT_EQUAL(FALSE, fd.isNegative());