Fix 32-bit builds under -Wpedantic - #188
Conversation
Two separate errors block every ILP32 target, and they share one cause:
uint64_t is unsigned long on LP64 but unsigned long long on ILP32, and
long long is not part of C90 or C++98.
utest.h:1446: error: integer constant is too large for 'unsigned long'
[-Werror=long-long]
utest.h: error: ISO C++98 does not support the 'll' gnu_printf length
modifier [-Werror=format=]
The constants 0x7fffffffffffffffu and 0x7ff0000000000000u fit in unsigned long
on LP64 and need unsigned long long on ILP32. Building them in the target type
avoids any oversized literal, and every operand then fits in 32 bits:
both.u &= ~((utest_uint64_t)1 << 63);
return both.u > ((utest_uint64_t)0x7ff00000 << 32);
UTEST_PRIu64 and UTEST_PRId64 expand to PRIu64/PRId64 from <inttypes.h>, which
is "lu"/"ld" on LP64 — accepted by C++98 — but "llu"/"lld" on ILP32, which is
not. Printing through double with "%.0f" sidesteps the length modifier
entirely. Integers below 2^53 print exactly, which is 104 days expressed in
nanoseconds, so nothing the framework reports can be truncated. The MSVC
"I64d"/"I64u" special case is no longer needed either.
The two must be fixed together: each one alone still blocks the build.
Verified:
i386 Linux, GCC 12 build failed -> 441/441
AIX 7.2 TL04 on POWER8, GCC's default build failed -> 441/441
32-bit mode
Unchanged where it already worked:
x86_64 Linux, glibc and musl 441/441
AIX 7.2 on POWER8, 64-bit 441/441
Windows MSVC 19.44, x86 and x64, 387/387
dynamic and static CRT
clang with AddressSanitizer, UBSan 441/441
NetBSD 9.4 and 10.1, OpenBSD 7.9, compile clean
FreeBSD 14.3 (cross, vendor sysroots)
The MSVC rows matter most here, since the change replaces the "I64d" branch as
well as the <inttypes.h> one.
|
One more platform this fixes, which I had not realised when opening the PR. CI on a NetBSD 10.1 runner fails to build utest.h in C++98 for a reason unrelated
#if !defined(__cplusplus) || defined(__STDC_FORMAT_MACROS) || \
(__cplusplus >= 201103L)
The That makes three distinct failure modes with one fix: printing through Observed on a NetBSD 10.1 VM via |
| #if defined(_MSC_VER) && (_MSC_VER < 1920) | ||
| #define UTEST_PRId64 "I64d" | ||
| #define UTEST_PRIu64 "I64u" | ||
| #define UTEST_PRId64 ".0f" |
There was a problem hiding this comment.
Eh this is trying to print integers with floats? Won't large integer values that are converted to doubles not be represented correctly and thus print wrong?
There was a problem hiding this comment.
You're right that it's lossy in principle. The question is where the limit falls.
Everything printed through these macros is a test count, a test index, or a duration in nanoseconds. double is exact below 2^53, which is about 104 days for a single test case. Nothing this framework reports can reach that.
The obvious alternative is worse: "lu"/"ld" with a cast truncates at 2^32 nanoseconds, or 4.3 seconds — a duration tests hit all the time.
There is a second reason for not keeping PRIu64, unrelated to 32-bit. NetBSD's <sys/inttypes.h> only exposes the PRI macros in C++ under __STDC_FORMAT_MACROS or from C++11 on:
#if !defined(__cplusplus) || defined(__STDC_FORMAT_MACROS) || \
(__cplusplus >= 201103L)
utest.h includes <inttypes.h> with neither, so test98.cpp at -std=gnu++98 does not build on NetBSD 10.1 at all — on a 64-bit machine. Printing through double removes that dependency as a side effect.
If you'd rather not print integers through a float at any bound, the lossless option is to format the 64-bit value into a small buffer by hand and print it with %s: no length modifier, so still C90/C++98 clean, no PRI dependency, and no precision limit. It's about 15 lines. Happy to do that instead — your call.
There was a problem hiding this comment.
I think I'd rather you have a fallback for platforms that don't support 64-bit integer printing (they are the minority), and keep the code as is otherwise.
The macOS jobs are red: AppleClang compiles the C++ sources with -Weverything
-Werror, and -Weverything includes -Wold-style-cast, which the three C casts
this PR introduced trip on.
utest.h:1447:15: error: use of old-style cast [-Werror,-Wold-style-cast]
utest.h:1460:15: error: use of old-style cast
utest.h:1461:20: error: use of old-style cast
UTEST_CAST already exists for exactly this and expands to static_cast in C++
and to a plain cast otherwise.
Reproduced with clang and the project's own flags before and after: the three
diagnostics are gone and the suite builds and runs green at 2161 tests. The
32-bit fix this PR exists for is unaffected, measured through subprocess.h's
suite at -m32, where main still fails on the oversized constants and both
revisions of this branch build and pass 443 tests.
Note the ILP32 defect is invisible to this repository's own CI, which builds
without -Wpedantic; it shows up in subprocess.h, which vendors this header.
Reworked per review: keep PRId64/PRIu64 as the normal path and add a fallback only for the platforms that cannot print 64-bit integers, rather than changing how every target prints. The call sites pass integers again. UTEST_INT64_ARG/UTEST_UINT64_ARG expand to nothing on the normal path and to a double cast on the fallback, so there is still one call site per message. The fallback triggers where uint64_t is not unsigned long and the language has no ll length modifier — ILP32 under C90 or C++98. _MSC_VER is listed explicitly: Windows is LLP64, so unsigned long is 32 bits even on x64, and MSVC reports __cplusplus as 199711L without /Zc:__cplusplus, so neither of the other tests recognises it. Measured before adding it, MSVC took the fallback in all four of x86/x64 × C/C++. NetBSD needs no fallback at all: defining __STDC_FORMAT_MACROS before <inttypes.h> restores the PRI macros for C++98, which is what that header documents. Checked that it draws no -Wreserved-macro-identifier under clang. Which branch is live was measured, not assumed — a fallback that triggers everywhere would also build everywhere: x86_64, C gnu89/c99 and C++ gnu++98/c++17 native "lu"/"ld" i386, C gnu89 and C++ gnu++98 fallback ".0f" i386, C c99 and C++ c++11 native "llu"/"lld" MSVC 19.44 x86 and x64, C and C++ native "llu"/"lld" AIX 7.3 POWER10, 32- and 64-bit native "llu"/"lu" Suite green under gcc and under clang with -Weverything -Werror (2161), and under MSVC on both architectures (2173 run, 10 intentional Todo skips). Verified on real AIX 7.3 / POWER10, where GCC defaults to 32-bit and this PR matters most. With sheredom#189 applied for platform support, subprocess.h's suite builds and passes 443 tests at 32- and at 64-bit; without this change the 32-bit build still fails on the oversized constants. Both paths coexist there in one binary, since that suite compiles some translation units at -std=gnu89 and -std=gnu++98 and the rest at newer standards. NetBSD's C++98 path is the one thing not reproducible locally — sheredom#113's netbsd 10.1 job covers it.
|
Done — Two corrections the measurement forced:
Which arm is live was checked per target at runtime rather than assumed from "it compiles": One thing that may look redundant: |
|
Status of all seven open PRs across both repos collected in |
utest.h does not build for any ILP32 target under the warning settings projects
commonly enable, and in C++98 it does not build on NetBSD at all. Both come from
<inttypes.h>:uint64_tisunsigned longon LP64 butunsigned long longonILP32, and
long longis not part of C90 or C++98.What this does
The constants.
0x7fffffffffffffffuand0x7ff0000000000000uare built inthe target type, so no oversized literal appears and every operand fits in 32 bits:
The format macros. Following your review,
PRId64/PRIu64stay the normalpath and the
doublefallback applies only wherellis genuinely unavailable —ILP32 under C90 or C++98. Call sites pass their value through
UTEST_INT64_ARG/UTEST_UINT64_ARG, which expand to(x)on the normal path andto a
doublecast on the fallback, so there is still one call site per message.The MSVC
"I64d"/"I64u"branch is untouched._MSC_VERis named explicitly in that condition. Windows is LLP64, sounsigned longis 32 bits even on x64, and MSVC reports__cplusplusas199711Lwithout/Zc:__cplusplus. Without naming it, MSVC took the fallback inall four x86/x64 × C/C++ configurations — precisely the silent change your review
was about.
NetBSD.
__STDC_FORMAT_MACROSis defined before<inttypes.h>, which is whatthat header documents. The macro is already defined in utest.h, but at line 274 on
main— 65 lines after the file's only#include <inttypes.h>at line 209, andinside
#if defined(__linux__). It therefore cannot affect that include, andNetBSD never reaches it. With the define in the right place no fallback is needed
there at all.
macOS. The three casts use
UTEST_CAST; plain C casts tripped-Wold-style-castunder the-Weverything -Werrorthis project applies to itsC++ sources, which is what the macOS jobs caught on the first revision.
Verification
Which arm is actually live, checked at runtime per target rather than inferred
from a successful build — a fallback that triggers everywhere also compiles
everywhere, and
UTEST_PRIu64is a string literal either way:"lu"/"ld"".0f""llu"/"lld""llu"/"lld""llu"/"lu"On AIX, measured one variant at a time so the result is attributable to a single
change:
main#error Unsupported platform!#error Unsupported platform!#error Unsupported platform!#error Unsupported platform!llmodifierOn AIX this PR alone changes nothing, and I would rather be precise about
that than claim more:
utest_ns()rejects the platform before the 32-bit problemsare reached, so #189 gates everything there. On i386 Linux, where no platform
branch is involved, this PR alone takes the build from failing to 441 / 441.
Re-measured after the rework on the same POWER10 machine: 443 / 443 at both word
sizes with #189 and this PR, and the build still fails 32-bit with #189 alone, so
the control still bites. 443 rather than 441 because the vendoring project has
gained two tests since.
Nothing changed where it already worked: x86_64 Linux glibc and musl, MSVC 19.44
x86 and x64 with both CRTs, clang with UBSan, and compile-clean on NetBSD 9.4,
OpenBSD 7.9 and FreeBSD 14.3. Test counts come from a project that vendors this
header, so the same file is exercised across all of those targets.