Open tschwinge opened 1 year ago
I think either solution is ok. c_int
will not always resolve to an i32
depending on the target. I'm not sure how this will affect the testsuite on some weirder architectures. I think it's definitely more correct to declare printf
as returning an int32_t
than nothing at all, but I'm afraid we'll run into some weird build failures on some systems.
Given libc int printf(const char *restrict format, ...);
, I think my preference is to actually turn our prototypes into the most-correct (per my understanding) fn printf(format: *const c_char, ...) -> c_int;
, alongside with local definitions of c_char
and c_int
. The latter may then be removed once we've got them defined properly. That is:
+// See <https://github.com/Rust-GCC/gccrs/issues/1887> "Non-conforming `printf` prototypes in the GCC/Rust testsuite".
+pub type c_char = i8;
+pub type c_int = i32;
extern "C" {
- fn printf(s: *const i8, ...);
+ fn printf(format: *const c_char, ...) -> c_int;
}
Well, like here: https://docs.rs/libc/latest/libc/fn.printf.html.
Sounds good to you?
Yes, that looks good :) I like it. Do you want to do the PR or leave it as a good-first-pr
@tschwinge ?
Either way is fine for me -- but I'm not sure, is this a good-first-pr
given that it's just some repetitive, tedious editing?
Either way is fine for me -- but I'm not sure, is this a
good-first-pr
given that it's just some repetitive, tedious editing?
In that case, it might just be easier to sed
the whole testsuite and not care about pub type c_int = i32
, and just return i32
when declaring printf
.
Can we define __c_int
as a builtin type, like i32
or i8
? Perhaps gated behind a compiler flag.
Can we define
__c_int
as a builtin type, likei32
ori8
? Perhaps gated behind a compiler flag.
I think this could cause issues down the line. The definition is in the core
crate, we just need to get there :) for now I think we should do as we have done and include the necessary core
items for our testcases - in this case, type c_int = i32
Just for fun, I tried enabling GCC/Rust in my GCC/nvptx target testing (see https://gcc.gnu.org/wiki/nvptx).
As you'd hope, it generally works.
One issue, however, is that in our hacked-up
printf
test cases, we use:..., which GCC/nvptx turns into:
In contrast, the libc one is declared/defined as:
(That is, with the expected
int
return type; don't ask me why unsigned.u32
instead of signed.s32
, but apparently that's OK.)Therefore, all those execution tests FAIL:
Should I fix all those up like (a):
..., which should be correst most of all times, or (b):
..., to make that more obvious, or (c) something else?
(See https://doc.rust-lang.org/stable/std/ffi/type.c_int.html.)