[clang][test] Try to fix Sema/format-strings.c on i686#181800
[clang][test] Try to fix Sema/format-strings.c on i686#181800
Conversation
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) Changes#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: Full diff: https://github.com/llvm/llvm-project/pull/181800.diff 1 Files Affected:
diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c
index 07bac7095ee82..96159afefebd7 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -986,7 +986,7 @@ void test_promotion(void) {
void test_bool(_Bool b, _Bool* bp)
{
-#ifndef __arm__
+#ifdef __LP64__
printf("%zu", b); // expected-warning-re{{format specifies type 'size_t' (aka '{{.+}}') but the argument has type '_Bool'}}
printf("%td", b); // expected-warning-re{{format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type '_Bool'}}
#else
|
e3b2472 to
b71986e
Compare
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
54ca20a to
9df0b64
Compare
clang/test/Sema/format-strings.c
Outdated
| void test_bool(_Bool b, _Bool* bp) | ||
| { | ||
| #ifndef __arm__ | ||
| #if defined(__LP64__) || defined(_WIN32) |
There was a problem hiding this comment.
This works for ARMv7 and AArch64, but I'm not sure about AIX.
Although I don't have AIX hardware to test this PR, I had a previous fix that failed on AIX because it has 64-bit integers and longs, so it seems to be ILP64 instead of LP64, and the warnings are displayed on it.
There was a problem hiding this comment.
Can we just hardcode all triples this test should run with and drop any run lines using the default triple?
There was a problem hiding this comment.
It seems a good option to me, to avoid unintentionally breaking other platforms. But I only fixed a failure on 32-bit Arm buildbots, caused by this test. I also don't know which triples this test should be run with.
Perhaps @YexuanXiao, @cor3ntin, and @zyn0217 can give better feedback on this.
There was a problem hiding this comment.
It looks like the Clang preprocessor defines convenient SIZEOF macros, so we can write this as #if __SIZEOF_INT__ != __SIZEOF_SIZE_T__. I think that should be robust.
I checked that this works for all of -triple=powerpc64-ibm-aix-xcoff, -triple=i686-unknown-linux-gnu, -triple=x86_64-pc-windows-msvc and -triple=i686-pc-windows-msvc.
The current check in this PR fails on -triple=i686-pc-windows-msvc.
|
I didn't realize at the time that these two lines of tests were invalid on so many platforms. To avoid potential failures, I think it's acceptable to limit them to triples known to produce warnings. |
clang/test/Sema/format-strings.c
Outdated
There was a problem hiding this comment.
Remove the #else.
It looks like the originally problematic triple was powerpc-ibm-aix, where the warning occurs despite the types having the same size, because they're still different types. We should just omit these no-warning lines to avoid the issue.
llvm#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: ``` ```
0b9d877 to
ba1a61a
Compare
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. # `----------------------------- ```
#180566 did this for 32bit arm, but this still breaks for us downstream on i686 with: