From 0da942bd52ffdd3621689fbc4bf3017e75b001e3 Mon Sep 17 00:00:00 2001 From: Fredrik Roubert Date: Fri, 12 Oct 2018 14:33:03 +0200 Subject: [PATCH] ICU-20080 Avoid strange compiler behaviour in ASSERT_EQUAL() macro. 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 | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/icu4c/source/test/intltest/dcfmapts.cpp b/icu4c/source/test/intltest/dcfmapts.cpp index 6a79bab8509..9fa0e3deee4 100644 --- source/test/intltest/dcfmapts.cpp +++ source/test/intltest/dcfmapts.cpp @@ -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());