[Sema] Fix format-strings test on AIX#180566
Conversation
On AIX, long has the same size as int.
|
@llvm/pr-subscribers-clang Author: Leandro Lupori (luporl) ChangesOn AIX, long has the same size as int. Full diff: https://github.com/llvm/llvm-project/pull/180566.diff 1 Files Affected:
diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c
index 3a2c2701cfcfc..0c13a70fa32eb 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -988,12 +988,14 @@ void test_promotion(void) {
void test_bool(_Bool b, _Bool* bp)
{
-#if SIZE_MAX != UINT_MAX
+ // Expect a warning if size_t/ptrdiff_t size is greater than a _Bool promoted
+ // to an int or if it might be a long int, with the same size of an int.
+#if SIZE_MAX > UINT_MAX || UINT_MAX == ULONG_MAX
printf("%zu", b); // expected-warning-re{{format specifies type 'size_t' (aka '{{.+}}') but the argument has type '_Bool'}}
#else
printf("%zu", b); // no-warning
#endif
-#if PTRDIFF_MAX != INT_MAX
+#if PTRDIFF_MAX > INT_MAX || INT_MAX == LONG_MAX
printf("%td", b); // expected-warning-re{{format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type '_Bool'}}
#else
printf("%td", b); // no-warning
|
The last change breaks ARMv7. Simplify things by skipping the problematic warnings only on 32-bit Arm.
|
The code formatting failure is a GitHub issue: |
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
|
Caught this failure a bit late for the release, so I will attempt to cherry-pick it. |
|
/pull-request #182755 |
|
Sorry to be late to the party but this change breaks tests on 32-bit x86 Linux: |
|
Can you please explain what the issue on AIX actually was? |
The issue on AIX was reported here: #178450 (comment) AFAIK, the problem is that on PPC64 AIX both integers and longs have 64 bits and size_t/ptrdiff_t are long. |
|
Looking at that buildbot, the triple it actually uses is powerpc-ibm-aix, even though the buildbot name is clang-ppc64-aix. And yes, for 32-bit AIX we have that size_t is long, which is the same size as int. I think the right fix for the issue is to just drop the no-warning lines, as I've suggested here: https://github.com/llvm/llvm-project/pull/181800/changes#r2840874528 If the sizes don't match there should definitely be a warning. If they do match, then it depends on the exact type definitions, and we shouldn't test either way. (Though at a higher level, if we ignore the immediate test issue here, IMHO, for this specific case of _Bool we should really be always warning regardless of what the underlying type definitions are. Using |
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` # .---command stderr------------ # | error: 'expected-warning' diagnostics expected but not seen: # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 990: format specifies type 'size_t' (aka '{{.+}}') but the argument has type '_Bool' # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 991: format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type '_Bool' # | 2 errors generated. # `----------------------------- ```
…81800) llvm/llvm-project#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` # .---command stderr------------ # | error: 'expected-warning' diagnostics expected but not seen: # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 990: format specifies type 'size_t' (aka '{{.+}}') but the argument has type '_Bool' # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 991: format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type '_Bool' # | 2 errors generated. # `----------------------------- ```
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` # .---command stderr------------ # | error: 'expected-warning' diagnostics expected but not seen: # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 990: format specifies type 'size_t' (aka '{{.+}}') but the argument has type '_Bool' # | File /builddir/build/BUILD/llvm-project/clang/test/Sema/format-strings.c Line 991: format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type '_Bool' # | 2 errors generated. # `----------------------------- ```
Simplify the conditional compilation and skip the problematic warnings only on 32-bit Arm.