test: use the clang definition for UNIT{32,64}_MAX #23651
Conversation
|
@swift-ci please test |
|
Build failed |
| @@ -10,9 +10,9 @@ | |||
| #define GL_RGB 0x1907 | |||
| #define GL_RGBA 0x1908 | |||
| #define EOF (-1) | |||
| #define UINT32_MAX 0xFFFFFFFFU | |||
| #define INT64_MAX 0x7FFFFFFFFFFFFFFFLL | |||
| #define UINT64_MAX 0xFFFFFFFFFFFFFFFFLL | |||
There was a problem hiding this comment.
Uh, hm. These were also being tested as edge-case constants with suffixes. Can you leave that in in some form?
There was a problem hiding this comment.
The clang definitions do have the suffixes as well:
clang -target x86_64-apple-macosx10.14 -x c -E /dev/null -dM -o - | grep -e __UINT32_MAX__ -e __INT64_MAX__ -e __UINT64_MAX__
#define __UINT32_MAX__ 4294967295U
#define __INT64_MAX__ 9223372036854775807LL
#define __UINT64_MAX__ 18446744073709551615ULL
Does the hex vs decimal encoding really matter?
There was a problem hiding this comment.
Ah, I see the difference ... you had left __UINT64_MAX__ as a signed value rather than unsigned. Was that intentional? Could've used a comment about that. However, the UINT32_MAX and INT64_MAX values seem identical to me (modulo spelling).
There was a problem hiding this comment.
I think these spellings were copied out of macOS's limits.h (well, not actually that, but somewhere in <sys/*/*.h>). The hex vs. decimal could also make a difference given that Swift could be looking at the spelling of the constant for a hint about the intended type. (It's not currently, but it could.)
There was a problem hiding this comment.
Hmm, actually, now Im not sure how this works on non-Windows. XOR_HIGH is xor-ing a signed and unsigned type, so you should get a signed type. How does that get cast?
error: cannot convert value of type 'Int64' to type 'CUnsignedLongLong' (aka 'UInt64') in coercion
There was a problem hiding this comment.
Again, it's not typed, so it has to pick what ^ should do. According to the "usual arithmetic conversions", the values can't be represented in the signed type, so the unsigned type should be preferred.
There was a problem hiding this comment.
Sorry, I wasn't very clear. The problem is that UINT32_MAX is suffixed with LL which is going to be promoted to unsigned long long by clang, except on Windows, because Microsoft does not permit that conversion and keeps it as a signed long long we have a type mismatch between the constants.
There was a problem hiding this comment.
That's important to know, then! I think Clang's behavior is more standards-compliant, but we should be testing that we're consistent with the platform.
There was a problem hiding this comment.
Yeah, its platform specific. I think that right now, I want to just make it unsigned for the existing tests and then add a specific test to ensure that the LL promotion happens in a target platform specific manner. Much like we did with the anonymous enumerations case :-).
Windows will keep the imported type as `signed long long` rather than `unsigned long long` as per the Microsoft compiler behaviour. This breaks the tests for this case. Unfortunately, this is one of those areas which must differ.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS platform |
Rather than defining our own type, use the clang definition which is
what Swift actually uses. This repairs the ClangImporter.macros and
ClangImporter.macro_literals tests.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.