From 68da9337b4a77f12d1e1e68ebff78c30581ca598 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Fri, 31 May 2024 03:23:26 -0700 Subject: [PATCH] Remove 'assert_c_type_sizes' test The test I am removing fails on 32-bit ARM, as pointers are 4-byte aligned. `upb_MessageValue` is a union. The alignment of a union is the max. alignment of any of its fields. Among the fields are 8-byte types like u64 and thus `upb_MessageValue` is 8-byte aligned even on 32-bit ARM. 4 != 8 and it fails. I am split on the usefulness of the test, as I find it somewhat unlikely to catch any divergence between the FFI and C types in the future. I can't imagine a practical scenario where upb_MessageValue would change in C code and this test would fail. IMO our actual unit & integration tests plus the IFTT lints are much better guards. We could fix the test by testing the alignment of u64 instead of *const c_void, which is always 8 bytes on the hw platforms we care about. PiperOrigin-RevId: 638976482 --- rust/upb.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/rust/upb.rs b/rust/upb.rs index 554af66bc3..1d5e396131 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -767,24 +767,3 @@ pub unsafe fn upb_Map_InsertAndReturnIfInserted( upb::MapInsertStatus::OutOfMemory => panic!("map arena is out of memory"), } } - -#[cfg(test)] -mod tests { - use super::*; - use googletest::prelude::*; - - #[test] - fn assert_c_type_sizes() { - // TODO: add these same asserts in C++. - use std::ffi::c_void; - use std::mem::{align_of, size_of}; - assert_that!( - size_of::(), - eq(size_of::<*const c_void>() + size_of::()) - ); - assert_that!(align_of::(), eq(align_of::<*const c_void>())); - - assert_that!(size_of::(), eq(size_of::<*const c_void>())); - assert_that!(align_of::(), eq(align_of::<*const c_void>())); - } -}