ctest: add foreign static test#4601
Conversation
6df4113 to
4455b42
Compare
ctest-next/templates/test.c
Outdated
| {%- if static_.rust_ty.contains("extern") +%} | ||
| typedef {{ static_.return_type }}; | ||
| ctest_static_ty__{{ static_.id }} ctest_static__{{ static_.id }}(void) { | ||
| return {{ static_.c_val }}; | ||
| } | ||
| {%- else +%} | ||
| typedef {{ static_.volatile_keyword }}{{ static_.c_mutable }}{{ static_.return_type }}; | ||
| ctest_static_ty__{{ static_.id }} ctest_static__{{ static_.id }}(void) { | ||
| return &{{ static_.c_val }}; | ||
| } |
There was a problem hiding this comment.
Since these return pointers, can they cast them on the C side and return void *? Rather than needing the more complex definition
There was a problem hiding this comment.
@mbyx this one still needs to be changed. It's also checking for an extern substring still.
The change should look something like #4594 (comment) for the function pointer part.
c0fa242 to
494e5e5
Compare
ctest-next/src/lib.rs
Outdated
| /* A simple helper to find any nested bare fn types, such as `Option<unsafe extern "C" fn()>` */ | ||
|
|
||
| struct BareFnFinder { |
There was a problem hiding this comment.
This can be a doc comment on the struct
ctest-next/src/template.rs
Outdated
| pub mutable: BoxStr, | ||
| pub c_mutable: BoxStr, |
There was a problem hiding this comment.
Nit: these can be &'static str
ctest-next/templates/test.c
Outdated
| {%- if static_.rust_ty.contains("extern") +%} | ||
| typedef {{ static_.return_type }}; | ||
| ctest_static_ty__{{ static_.id }} ctest_static__{{ static_.id }}(void) { | ||
| return {{ static_.c_val }}; | ||
| } | ||
| {%- else +%} | ||
| typedef {{ static_.volatile_keyword }}{{ static_.c_mutable }}{{ static_.return_type }}; | ||
| ctest_static_ty__{{ static_.id }} ctest_static__{{ static_.id }}(void) { | ||
| return &{{ static_.c_val }}; | ||
| } |
There was a problem hiding this comment.
@mbyx this one still needs to be changed. It's also checking for an extern substring still.
The change should look something like #4594 (comment) for the function pointer part.
ctest-next/templates/test.rs
Outdated
| let actual = unsafe { | ||
| std::mem::transmute::<_, *const ()>(*(&raw const {{ static_.id }})).addr() | ||
| }; | ||
| let expected = unsafe { | ||
| std::mem::transmute::<_, *const ()>(ctest_static__{{ static_.id }}()).addr() |
There was a problem hiding this comment.
ctest-next/templates/test.rs
Outdated
| let actual = unsafe { | ||
| std::mem::transmute::<_, *const ()>(*(&raw const {{ static_.id }})).addr() | ||
| }; | ||
| let expected = unsafe { | ||
| std::mem::transmute::<_, *const ()>(ctest_static__{{ static_.id }}()).addr() |
There was a problem hiding this comment.
Please add comments, this is tricky. Also, make sure this branch is covered in one of the tests.
494e5e5 to
148bb13
Compare
tgross35
left a comment
There was a problem hiding this comment.
LGTM! What happened part with special handling for functions?
148bb13 to
dcfb81a
Compare
|
Now it always returns a pointer to something, whether its a data item or a function pointer/array, etc. So the Rust side just checks for equality of addresses. I'm not sure if that's perfectly correct though. |
|
Ah, I didn't catch that - it will work but it technically isn't correct because of the function->data pointer cast. I don't think we need to deal with it now, could you make that a FIXME? We should enable warnings in the C builds at some point, at least during testing, |
19af932 to
2d8d548
Compare
tgross35
left a comment
There was a problem hiding this comment.
LGTM! Unfortunately CI is still blocked
2d8d548 to
c0b5f23
Compare
c0b5f23 to
1685c18
Compare
Pull Request is not mergeable
Description
Adds support for testing extern statics.
Also adds support for parsing any dimensional array in
make_cdecl, and makes it easier to extract ffi safe types fromOption.Sources
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI