rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
122 stars 18 forks source link

Generate layout tests using real `sizeof`/`alignof` #181

Closed ian-h-chamberlain closed 5 months ago

ian-h-chamberlain commented 6 months ago

Similar to the tests generated by bindgen, but using our actual toolchain to spit out the expected values instead of what libclang generates (bindgen's default behavior).

The full generated test can be downloaded from Actions: https://github.com/rust3ds/ctru-rs/actions/runs/9072691412/artifacts/1499437252

Here's a partial example:

#[test]
fn layout_test_tag_aptHookCookie() {
    assert_eq!(
        size_of!(tag_aptHookCookie),
        cpp ! (unsafe [] -> usize as "size_t" { return sizeof (tag_aptHookCookie) ; }),
        "{} != {}",
        stringify!(size_of!(tag_aptHookCookie)),
        stringify!(sizeof(tag_aptHookCookie)),
    );
    assert_eq!(
        align_of!(tag_aptHookCookie),
        cpp ! (unsafe [] -> usize as "size_t" { return alignof (tag_aptHookCookie) ; }),
        "{} != {}",
        stringify!(align_of!(tag_aptHookCookie)),
        stringify!(alignof(tag_aptHookCookie)),
    );
    assert_eq!(
        size_of!(tag_aptHookCookie::callback),
        cpp ! (unsafe [] -> usize as "size_t" { return sizeof (tag_aptHookCookie :: callback) ; }),
        "{} != {}",
        stringify!(size_of!(tag_aptHookCookie::callback)),
        stringify!(sizeof(tag_aptHookCookie::callback)),
    );
    assert_eq!(
        align_of!(tag_aptHookCookie::callback),
        cpp ! (unsafe [] -> usize as "size_t" { return alignof (tag_aptHookCookie :: callback) ; }),
        "{} != {}",
        stringify!(align_of!(tag_aptHookCookie::callback)),
        stringify!(alignof(tag_aptHookCookie::callback)),
    );
    assert_eq!(
        offset_of!(tag_aptHookCookie, callback),
        cpp ! (unsafe [] -> usize as "size_t" { return offsetof (tag_aptHookCookie , callback) ; }),
        "{} != {}",
        stringify!(offset_of!(tag_aptHookCookie, callback)),
        stringify!(offsetof(tag_aptHookCookie, callback)),
    );
}
ian-h-chamberlain commented 5 months ago

Other than for a couple of missing features, which aren't really needed, I feel like this addition has all that it needs to be an useful replacement for bindgen tests. Great job! 👏

This is an interesting point — for now, I've left the bindgen tests in place because I think they might still be useful to assert properties that clang thinks about the generated types... Especially given that only adds around 1 minute to CI time, and takes almost no effort on our part, do you think we should leave those alone and keep running them alongside these new tests?

Otherwise, I think this PR is pretty much good to go so I will probably merge once everything is passing 🚀

Meziu commented 5 months ago

This is an interesting point — for now, I've left the bindgen tests in place because I think they might still be useful to assert properties that clang thinks about the generated types... Especially given that only adds around 1 minute to CI time, and takes almost no effort on our part, do you think we should leave those alone and keep running them alongside these new tests?

Very much so, bindgen tests are still a pretty integral part in the binding generation routine, and they are still the "best" comparative case for the suite added here. I'd keep them around for as long as we feel fit for testing the new suite and tweaking the bindgen setup (which after all these past issues I don't trust to be precise enough :sweat_smile:).

Otherwise, I think this PR is pretty much good to go so I will probably merge once everything is passing 🚀

:+1: