[PATCH] D146492: Add new printNumber() for size_t
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 22 01:06:47 PDT 2023
jhenderson added a comment.
Thanks @junhee-yoo, the difference that Mac uses `unsigned long` not `unsigned long long` was the piece I was missing. I bet if you look at the `uint32_t` type you'll see it is defined as `unsigned int`. This will mean that the current `printNumber` overloads for Mac probably cover the following fundamental integer types:
- `char` (or possibly `signed char`) (from `int8_t`)
- `short` (from `int16_t`)
- `int` (from `int32_t`)
- `long long` (from `int64_t`)
and their unsigned equivalents from the `uint*_t` types.
The problem ultimately boils down to the fact that we've attempted to cover all sizes of integers with `printNumber` overloads, but haven't actually done so, because we've used the fixed-width integer types, rather than the fundamental C++ types. The latter are what are ultimately used in picking which function overload to use. I'd therefore like to propose the following changes:
1. Replace the existing `printNumber` integer type overloads with fundamental type versions (specifically `char`, `signed char`, `unsigned char`, `short`, `unsigned short`, `int`, `unsigned int`, `long`, `unsigned long`, `long long`, and `unsigned long long` (see https://en.cppreference.com/w/cpp/language/types for a good reference).
2. Revert any "fixes" (casts etc) that have been added to make the code work on Mac or other platforms, since they'll be unnecessary at this point and just clutter the code (and none of them were reviewed...).
I'd do this myself, but actually coding on LLVM falls outside my job remit these days. @paulkirth, it probably should be your responsibility as the person who originally introduced the breakage to coordinate this!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146492/new/
https://reviews.llvm.org/D146492
More information about the llvm-commits
mailing list