From f2cb18e26a184bdc7fb8aad85828e9e87985764b Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Mon, 22 Apr 2024 22:51:39 -0400 Subject: [PATCH 01/13] Initial pass of generated C layout tests --- ctru-sys/Cargo.toml | 4 +- ctru-sys/build.rs | 152 ++++++++++++++++++++++++++++++++-- ctru-sys/tests/layout_test.rs | 18 ++++ 3 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 ctru-sys/tests/layout_test.rs diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index 8d64a1ba..2c82d677 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -16,13 +16,15 @@ edition = "2021" libc = { version = "0.2.121", default-features = false } [build-dependencies] -bindgen = { version = "0.65.1", features = ["experimental"] } +bindgen = { version = "0.66.1", features = ["experimental"] } cc = "1.0" +cpp_build = "0.5.9" doxygen-rs = "0.4.2" itertools = "0.11.0" which = "4.4.0" [dev-dependencies] +cpp = "0.5.9" shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 71ad909c..7623a171 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -1,19 +1,47 @@ -use bindgen::callbacks::ParseCallbacks; -use bindgen::{Builder, RustTarget}; +use bindgen::callbacks::{DeriveInfo, FieldInfo, ParseCallbacks}; +use bindgen::{Builder, FieldVisibilityKind, RustTarget}; use itertools::Itertools; +use std::io::{self, Write}; +use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; use std::env; use std::error::Error; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; +use std::rc::Rc; + +#[derive(Debug, Default)] +struct StructInfo { + fields: HashMap>, + names: HashSet, +} #[derive(Debug)] -struct CustomCallbacks; +struct CustomCallbacks(Rc>); impl ParseCallbacks for CustomCallbacks { fn process_comment(&self, comment: &str) -> Option { Some(doxygen_rs::transform(comment)) } + + fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { + self.0.borrow_mut().names.insert(info.name.to_string()); + Vec::new() + } + + // We don't actually ever change visibility, but this allows us to keep track + // of all the fields in the structs bindgen processes. + fn field_visibility(&self, info: FieldInfo<'_>) -> Option { + self.0 + .borrow_mut() + .fields + .entry(info.type_name.to_string()) + .or_default() + .insert(info.field_name.to_string()); + + None + } } fn main() { @@ -65,6 +93,9 @@ fn main() { )); let errno_header = system_include.join("errno.h"); + let struct_info = Rc::default(); + let callbacks = CustomCallbacks(Rc::clone(&struct_info)); + // Build libctru bindings let bindings = Builder::default() .header(ctru_header.to_str().unwrap()) @@ -118,7 +149,7 @@ fn main() { // gcc, so bindgen will generate enums with the proper sizes. "-fshort-enums", ]) - .parse_callbacks(Box::new(CustomCallbacks)) + .parse_callbacks(Box::new(callbacks)) .generate() .expect("unable to generate bindings"); @@ -131,8 +162,8 @@ fn main() { let ar = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-ar"); cc::Build::new() - .compiler(cc) - .archiver(ar) + .compiler(&cc) + .archiver(&ar) .include(&include_path) .file(out_dir.join("libctru_statics_wrapper.c")) .flag("-march=armv6k") @@ -142,6 +173,21 @@ fn main() { .flag("-mtp=soft") .flag("-Wno-deprecated-declarations") .compile("ctru_statics_wrapper"); + + let struct_info = struct_info.borrow(); + let generated_test_file = struct_info.build_layout_tests().unwrap(); + + cpp_build::Config::default() + .compiler(cc) + .archiver(ar) + .include(include_path) + .flag("-march=armv6k") + .flag("-mtune=mpcore") + .flag("-mfloat-abi=hard") + .flag("-mfpu=vfp") + .flag("-mtp=soft") + .flag("-Wno-deprecated-declarations") + .build(generated_test_file); } fn get_gcc_version(path_to_gcc: PathBuf) -> String { @@ -232,7 +278,7 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> { } }; - for line in String::from_utf8_lossy(&stdout).trim().split('\n') { + for line in String::from_utf8_lossy(&stdout).lines() { let Some((_pkg, file)) = line.split_once(char::is_whitespace) else { println!("cargo:warning=unexpected line from pacman query: {line:?}"); continue; @@ -243,3 +289,95 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> { Ok(()) } + +impl StructInfo { + fn build_layout_tests(&self) -> io::Result { + let output_file = PathBuf::from(env::var("OUT_DIR").unwrap()).join("layout_test.rs"); + let mut file = std::fs::File::create(&output_file)?; + + writeln!( + file, + r#" +use cpp::cpp; +use ctru_sys::*; + +cpp! {{{{ + #include <3ds.h> +}}}} +"#, + )?; + + for (strukt, fields) in &self.fields { + if strukt.contains("bindgen") || !self.names.contains(strukt) { + // We don't have an easy way to map the mangled name back to the original name, + // so we will just skip testing any structs with names generated by bindgen. + // + // If we needed to we could maybe hardcode a map for the ones we care about. + continue; + } + + if let "sigevent" + | "siginfo_t" + | "sigval" + | "bintime" + | "fd_set" + | "pthread_rwlock_t" + | "ExHeader_Arm11CoreInfo" + | "ExHeader_Arm11StorageInfo" + | "ExHeader_SystemInfoFlags" + | "FS_ExtSaveDataInfo" + | "FS_SystemSaveDataInfo" + | "FS_ProgramInfo" + | "Y2RU_ConversionParams" = strukt.as_str() + { + // Some of these are only forward declared (in stdlibs), or have bitfields, etc. + // If we wanted to be more precise we could check specific fields in + // these instead of just skipping the whole struct. + continue; + } + + // TODO: May need to mangle rust names to match what bindgen spits out... + writeln!( + file, + r#" +#[test] +fn {strukt}_layout() {{ + assert_eq!( + std::mem::size_of::<{strukt}>(), + cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}); }}), + ); + + assert_eq!( + std::mem::align_of::<{strukt}>(), + cpp!(unsafe [] -> usize as "size_t" {{ return alignof({strukt}); }}), + ); +"#, + )?; + + for field in fields { + if field.contains("bindgen") { + // Similar to struct names, just skip these ones + continue; + } + + // HACK: This will break if some struct actually has a field called `type_` + let c_field = if field == "type_" { "type" } else { field }; + + // TODO: also check field size + align if reasonably feasible + writeln!( + file, + r#" + assert_eq!( + std::mem::offset_of!({strukt}, {field}), + cpp!(unsafe [] -> usize as "size_t" {{ return offsetof({strukt}, {c_field}); }}), + ); +"#, + )?; + } + + writeln!(file, "}}")?; + } + + Ok(output_file) + } +} diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs new file mode 100644 index 00000000..881040de --- /dev/null +++ b/ctru-sys/tests/layout_test.rs @@ -0,0 +1,18 @@ +//! This is a stub for the generated layout test. We use bindgen callbacks along +//! with the [`cpp`] crate to compile actual `sizeof` and `alignof` calls in C, +//! as opposed to bindgen's generated layout tests which use hardcoded size literals. +//! +//! This should help ensure that the generated bindings are correct for the actual +//! ABI used by libctru and the devkitARM toolchain, instead of just what libclang +//! thinks they should be at bindgen time. + +#![allow(non_snake_case)] +#![feature(custom_test_frameworks)] +#![test_runner(test_runner::run_gdb)] + +extern crate shim_3ds; + +// TODO: might want to move this into a test crate so we can avoid compiling it +// for non-test builds? Idk if there's a reasonable way to do it though. + +include!(concat!(env!("OUT_DIR"), "/layout_test.rs")); From 1b1676859e8bcca5e0ca345de85a1580595f4c5b Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Tue, 23 Apr 2024 00:40:10 -0400 Subject: [PATCH 02/13] Also test size/align of struct/union fields --- ctru-sys/build.rs | 30 ++++++++++++++++++++++++++---- ctru-sys/tests/layout_test.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 7623a171..fc72bf95 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -337,11 +337,11 @@ cpp! {{{{ } // TODO: May need to mangle rust names to match what bindgen spits out... - writeln!( + write!( file, r#" #[test] -fn {strukt}_layout() {{ +fn layout_test_{strukt}() {{ assert_eq!( std::mem::size_of::<{strukt}>(), cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}); }}), @@ -363,14 +363,34 @@ fn {strukt}_layout() {{ // HACK: This will break if some struct actually has a field called `type_` let c_field = if field == "type_" { "type" } else { field }; - // TODO: also check field size + align if reasonably feasible - writeln!( + write!( file, r#" assert_eq!( std::mem::offset_of!({strukt}, {field}), cpp!(unsafe [] -> usize as "size_t" {{ return offsetof({strukt}, {c_field}); }}), ); + assert_eq!( + align_of_field!({strukt}, {field}), + cpp!(unsafe [] -> usize as "size_t" {{ return alignof({strukt}::{c_field}); }}), + ); +"#, + )?; + + if let ("romfs_dir", "name") | ("romfs_file", "name") | ("sockaddr", "sa_data") = + (strukt.as_str(), field.as_str()) + { + // These are variable length arrays, so we can't use sizeof() + continue; + } + + write!( + file, + r#" + assert_eq!( + size_of_field!({strukt}, {field}), + cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}::{c_field}); }}), + ); "#, )?; } @@ -378,6 +398,8 @@ fn {strukt}_layout() {{ writeln!(file, "}}")?; } + // TODO: we could probably rustfmt here + Ok(output_file) } } diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs index 881040de..2d7aa186 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout_test.rs @@ -15,4 +15,32 @@ extern crate shim_3ds; // TODO: might want to move this into a test crate so we can avoid compiling it // for non-test builds? Idk if there's a reasonable way to do it though. +fn size_of_ret(_f: F) -> usize +where + F: FnOnce(T) -> U, +{ + std::mem::size_of::() +} + +macro_rules! size_of_field { + ($ty:ty, $field:ident) => {{ + #[allow(unused_unsafe)] + size_of_ret(|t: $ty| unsafe { t.$field }) + }}; +} + +fn align_of_ret(_f: F) -> usize +where + F: FnOnce(T) -> U, +{ + std::mem::align_of::() +} + +macro_rules! align_of_field { + ($ty:ty, $field:ident) => {{ + #[allow(unused_unsafe)] + align_of_ret(|t: $ty| unsafe { t.$field }) + }}; +} + include!(concat!(env!("OUT_DIR"), "/layout_test.rs")); From 2c40e69a2d0354c80025285aaa535fae3bd504f0 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Thu, 25 Apr 2024 22:55:58 -0400 Subject: [PATCH 03/13] Big refactor to use quote and separate crate --- Cargo.toml | 10 +- binding-helpers/Cargo.toml | 14 + binding-helpers/src/gen.rs | 155 +++++++++++ binding-helpers/src/lib.rs | 37 +++ ctru-sys/Cargo.toml | 10 +- ctru-sys/build.rs | 266 +++++-------------- ctru-sys/tests/{layout_test.rs => layout.rs} | 31 +-- 7 files changed, 294 insertions(+), 229 deletions(-) create mode 100644 binding-helpers/Cargo.toml create mode 100644 binding-helpers/src/gen.rs create mode 100644 binding-helpers/src/lib.rs rename ctru-sys/tests/{layout_test.rs => layout.rs} (60%) diff --git a/Cargo.toml b/Cargo.toml index d5e7af3a..af85a6a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,15 +1,23 @@ [workspace] +<<<<<<< HEAD members = ["ctru-rs", "ctru-sys", "test-runner"] +======= +members = ["binding-helpers", "ctru-rs", "ctru-sys"] +>>>>>>> 2c06946 (Big refactor to use quote and separate crate) default-members = ["ctru-rs", "ctru-sys"] resolver = "2" -[patch.'https://github.com/rust3ds/ctru-rs'] +[patch.'https://github.com/rust3ds/ctru-rs.git'] # Make sure all dependencies use the local packages. This is needed for things # like pthread-3ds that rely on ctru-sys, and test-runner which relies on ctru-rs ctru-rs = { path = "ctru-rs" } ctru-sys = { path = "ctru-sys" } +<<<<<<< HEAD test-runner = { path = "test-runner" } # This was the previous git repo for test-runner [patch."https://github.com/rust3ds/test-runner"] test-runner = { path = "test-runner" } +======= +binding-helpers = { path = "binding-helpers" } +>>>>>>> 2c06946 (Big refactor to use quote and separate crate) diff --git a/binding-helpers/Cargo.toml b/binding-helpers/Cargo.toml new file mode 100644 index 00000000..031eeefc --- /dev/null +++ b/binding-helpers/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "binding-helpers" +description = "A helper crate for testing Rust bindings' definitions against their C/C++ counterparts." +version = "0.1.0" +edition = "2021" + +[dependencies] +bindgen = "*" +proc-macro2 = "1.0.81" +quote = "1.0.36" +rust-format = { version = "0.3.4", features = ["token_stream"] } + +# [build-dependencies]cpp = "0.5.9" +# cpp_build = "0.5.9" diff --git a/binding-helpers/src/gen.rs b/binding-helpers/src/gen.rs new file mode 100644 index 00000000..7c9d3763 --- /dev/null +++ b/binding-helpers/src/gen.rs @@ -0,0 +1,155 @@ +use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; +use std::fmt::Display; +use std::io; +use std::path::Path; +use std::rc::Rc; + +use bindgen::callbacks::{DeriveInfo, FieldInfo, ParseCallbacks}; +use bindgen::FieldVisibilityKind; +use proc_macro2::TokenStream; +use quote::{format_ident, quote}; +use rust_format::{Formatter, RustFmt}; + +#[derive(Default, Debug)] +struct StructInfo { + fields: HashMap>, + names: HashSet, +} + +#[derive(Default, Debug)] +pub struct LayoutTests(Rc>); + +#[derive(Debug)] +pub enum Error { + Io(io::Error), + Format(rust_format::Error), +} + +impl Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Error::Io(io) => write!(f, "IO error: {io}"), + Error::Format(fmt) => write!(f, "Format error: {fmt}"), + } + } +} + +impl From for Error { + fn from(e: io::Error) -> Self { + Self::Io(e) + } +} + +impl From for Error { + fn from(e: rust_format::Error) -> Self { + Self::Format(e) + } +} + +impl LayoutTests { + pub fn new() -> (Self, Self) { + let this = Self::default(); + + (Self(Rc::clone(&this.0)), this) + } + + pub fn generate_layout_tests(&self, output_path: impl AsRef) -> Result<(), Error> { + let mut output = Vec::new(); + + for (struct_name, fields) in &self.0.borrow().fields { + if struct_name.contains("bindgen") { + continue; + } + output.push(struct_layout_test(struct_name, fields)); + } + + let tokens = quote! { #(#output)* }; + + // We can't use quote! here because it will reformat <> includes badly + // and also doesn't seem to play nice with whatever cpp! expands to. + let header = " + use cpp::cpp; + + cpp! {{ + #include + #include <3ds.h> + }} + "; + + std::fs::write(&output_path, String::from(header) + &tokens.to_string())?; + RustFmt::default().format_file(output_path)?; + + Ok(()) + } +} + +fn struct_layout_test( + struct_name: &str, + field_names: &HashSet, +) -> proc_macro2::TokenStream { + let name = format_ident!("{struct_name}"); + + let test_name = format_ident!("layout_test_{struct_name}"); + + let mut field_tests = Vec::new(); + field_tests.push(assert_eq(quote!(size_of!(#name)), quote!(sizeof(#name)))); + field_tests.push(assert_eq(quote!(align_of!(#name)), quote!(alignof(#name)))); + + for field in field_names { + if field.contains("bindgen") { + continue; + } + + let field = if field == "type_" { "type" } else { field }; + + let field = format_ident!("{field}"); + field_tests.push(assert_eq( + quote!(size_of!(#name::#field)), + quote!(sizeof(#name::#field)), + )); + field_tests.push(assert_eq( + quote!(align_of!(#name::#field)), + quote!(alignof(#name::#field)), + )); + } + + quote! { + #[test] + fn #test_name() { + #(#field_tests);* + } + } +} + +fn assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { + quote! { + assert_eq!( + #rust_lhs, + cpp!(unsafe [] -> usize as "size_t" { return #cpp_rhs; }), + "{} != {}", + stringify!(#rust_lhs), + stringify!(#cpp_rhs), + ); + } +} + +impl ParseCallbacks for LayoutTests { + fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { + self.0.borrow_mut().names.insert(info.name.to_string()); + Vec::new() + } + + // We don't actually ever change visibility, but this allows us to keep track + // of all the fields in the structs bindgen processes. + fn field_visibility(&self, info: FieldInfo<'_>) -> Option { + self.0 + .borrow_mut() + .fields + .entry(info.type_name.to_string()) + .or_default() + .insert(info.field_name.to_string()); + + None + } +} diff --git a/binding-helpers/src/lib.rs b/binding-helpers/src/lib.rs new file mode 100644 index 00000000..1bd61038 --- /dev/null +++ b/binding-helpers/src/lib.rs @@ -0,0 +1,37 @@ +pub mod gen; + +#[macro_export] +macro_rules! size_of { + ($ty:ident::$field:ident) => {{ + $crate::size_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::size_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::size_of_val(&$expr) + }; +} + +#[macro_export] +macro_rules! align_of { + ($ty:ident::$field:ident) => {{ + $create::align_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::align_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::align_of_val(&$expr) + }; +} + +#[doc(hidden)] +pub fn size_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::size_of::() +} + +#[doc(hidden)] +pub fn align_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::align_of::() +} diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index 2c82d677..b0e8527e 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -12,6 +12,14 @@ license = "Zlib" links = "ctru" edition = "2021" +[features] +layout-tests = [] + +[[test]] +path = "tests/layout.rs" +name = "layout" +required-features = ["layout-tests"] + [dependencies] libc = { version = "0.2.121", default-features = false } @@ -22,9 +30,9 @@ cpp_build = "0.5.9" doxygen-rs = "0.4.2" itertools = "0.11.0" which = "4.4.0" +binding-helpers = { git = "https://github.com/rust3ds/ctru-rs.git" } [dev-dependencies] -cpp = "0.5.9" shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index fc72bf95..492c750e 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -1,47 +1,22 @@ -use bindgen::callbacks::{DeriveInfo, FieldInfo, ParseCallbacks}; -use bindgen::{Builder, FieldVisibilityKind, RustTarget}; +use bindgen::callbacks::ParseCallbacks; +use bindgen::{Builder, RustTarget}; +use binding_helpers::gen::LayoutTests; use itertools::Itertools; use std::io::{self, Write}; -use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; use std::env; use std::error::Error; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; use std::rc::Rc; -#[derive(Debug, Default)] -struct StructInfo { - fields: HashMap>, - names: HashSet, -} - #[derive(Debug)] -struct CustomCallbacks(Rc>); +struct CustomCallbacks; impl ParseCallbacks for CustomCallbacks { fn process_comment(&self, comment: &str) -> Option { Some(doxygen_rs::transform(comment)) } - - fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { - self.0.borrow_mut().names.insert(info.name.to_string()); - Vec::new() - } - - // We don't actually ever change visibility, but this allows us to keep track - // of all the fields in the structs bindgen processes. - fn field_visibility(&self, info: FieldInfo<'_>) -> Option { - self.0 - .borrow_mut() - .fields - .entry(info.type_name.to_string()) - .or_default() - .insert(info.field_name.to_string()); - - None - } } fn main() { @@ -93,8 +68,47 @@ fn main() { )); let errno_header = system_include.join("errno.h"); - let struct_info = Rc::default(); - let callbacks = CustomCallbacks(Rc::clone(&struct_info)); + // Compile static inline fns wrapper + let cc = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-gcc"); + let cpp = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-g++"); + let ar = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-ar"); + + let mut builder = cc::Build::new(); + builder + .compiler(cc) + .archiver(ar) + .include(&include_path) + .define("ARM11", None) + .define("__3DS__", None) + .flag("-march=armv6k") + .flag("-mtune=mpcore") + .flag("-mfloat-abi=hard") + .flag("-mfpu=vfp") + .flag("-mtp=soft") + .flag("-Wno-deprecated-declarations"); + + let clang = builder + .clone() + .compiler("clang") + // bindgen uses clang, so we need to tell it where devkitARM sysroot / libs are: + .flag("--sysroot") + .flag(sysroot.to_str().unwrap()) + .flag("-isystem") + .flag(system_include.to_str().unwrap()) + .flag("-isystem") + .flag(gcc_include.to_str().unwrap()) + // Fun fact: C compilers are allowed to represent enums as the smallest + // integer type that can hold all of its variants, meaning that enums are + // allowed to be the size of a `c_short` or a `c_char` rather than the size + // of a `c_int`. Some of libctru's structs contain enums that depend on + // this narrowing property for size and alignment purposes. + // + // Passing this flag to clang gives approximately the same behavior as + // gcc, so bindgen will generate enums with the proper sizes. + .flag("-fshort-enums") + .get_compiler(); + + let (test_gen_callbacks, test_generator) = LayoutTests::new(); // Build libctru bindings let bindings = Builder::default() @@ -123,33 +137,9 @@ fn main() { .derive_default(true) .wrap_static_fns(true) .wrap_static_fns_path(out_dir.join("libctru_statics_wrapper")) - .clang_args([ - "--target=arm-none-eabi", - "--sysroot", - sysroot.to_str().unwrap(), - "-isystem", - system_include.to_str().unwrap(), - "-isystem", - gcc_include.to_str().unwrap(), - "-I", - include_path.to_str().unwrap(), - "-mfloat-abi=hard", - "-march=armv6k", - "-mtune=mpcore", - "-mfpu=vfp", - "-DARM11", - "-D__3DS__", - // Fun fact: C compilers are allowed to represent enums as the smallest - // integer type that can hold all of its variants, meaning that enums are - // allowed to be the size of a `c_short` or a `c_char` rather than the size - // of a `c_int`. Some of libctru's structs contain enums that depend on - // this narrowing property for size and alignment purposes. - // - // Passing this flag to clang gives approximately the same behavior as - // gcc, so bindgen will generate enums with the proper sizes. - "-fshort-enums", - ]) - .parse_callbacks(Box::new(callbacks)) + .clang_args(clang.args().iter().map(|s| s.to_str().unwrap())) + .parse_callbacks(Box::new(CustomCallbacks)) + .parse_callbacks(Box::new(test_gen_callbacks)) .generate() .expect("unable to generate bindings"); @@ -157,37 +147,27 @@ fn main() { .write_to_file(out_dir.join("bindings.rs")) .expect("Couldn't write bindings!"); - // Compile static inline fns wrapper - let cc = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-gcc"); - let ar = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-ar"); - - cc::Build::new() - .compiler(&cc) - .archiver(&ar) - .include(&include_path) + builder .file(out_dir.join("libctru_statics_wrapper.c")) - .flag("-march=armv6k") - .flag("-mtune=mpcore") - .flag("-mfloat-abi=hard") - .flag("-mfpu=vfp") - .flag("-mtp=soft") - .flag("-Wno-deprecated-declarations") .compile("ctru_statics_wrapper"); - let struct_info = struct_info.borrow(); - let generated_test_file = struct_info.build_layout_tests().unwrap(); - - cpp_build::Config::default() - .compiler(cc) - .archiver(ar) - .include(include_path) - .flag("-march=armv6k") - .flag("-mtune=mpcore") - .flag("-mfloat-abi=hard") - .flag("-mfpu=vfp") - .flag("-mtp=soft") - .flag("-Wno-deprecated-declarations") - .build(generated_test_file); + if env::var("CARGO_FEATURE_LAYOUT_TESTS").is_ok() { + let test_file = out_dir.join("layout_tests.rs"); + test_generator + .generate_layout_tests(&test_file) + .expect("Couldn't write layout tests!"); + + cpp_build::Config::default() + .compiler(cpp) + .include(include_path) + .flag("-march=armv6k") + .flag("-mtune=mpcore") + .flag("-mfloat-abi=hard") + .flag("-mfpu=vfp") + .flag("-mtp=soft") + .flag("-Wno-deprecated-declarations") + .build(test_file); + } } fn get_gcc_version(path_to_gcc: PathBuf) -> String { @@ -289,117 +269,3 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> { Ok(()) } - -impl StructInfo { - fn build_layout_tests(&self) -> io::Result { - let output_file = PathBuf::from(env::var("OUT_DIR").unwrap()).join("layout_test.rs"); - let mut file = std::fs::File::create(&output_file)?; - - writeln!( - file, - r#" -use cpp::cpp; -use ctru_sys::*; - -cpp! {{{{ - #include <3ds.h> -}}}} -"#, - )?; - - for (strukt, fields) in &self.fields { - if strukt.contains("bindgen") || !self.names.contains(strukt) { - // We don't have an easy way to map the mangled name back to the original name, - // so we will just skip testing any structs with names generated by bindgen. - // - // If we needed to we could maybe hardcode a map for the ones we care about. - continue; - } - - if let "sigevent" - | "siginfo_t" - | "sigval" - | "bintime" - | "fd_set" - | "pthread_rwlock_t" - | "ExHeader_Arm11CoreInfo" - | "ExHeader_Arm11StorageInfo" - | "ExHeader_SystemInfoFlags" - | "FS_ExtSaveDataInfo" - | "FS_SystemSaveDataInfo" - | "FS_ProgramInfo" - | "Y2RU_ConversionParams" = strukt.as_str() - { - // Some of these are only forward declared (in stdlibs), or have bitfields, etc. - // If we wanted to be more precise we could check specific fields in - // these instead of just skipping the whole struct. - continue; - } - - // TODO: May need to mangle rust names to match what bindgen spits out... - write!( - file, - r#" -#[test] -fn layout_test_{strukt}() {{ - assert_eq!( - std::mem::size_of::<{strukt}>(), - cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}); }}), - ); - - assert_eq!( - std::mem::align_of::<{strukt}>(), - cpp!(unsafe [] -> usize as "size_t" {{ return alignof({strukt}); }}), - ); -"#, - )?; - - for field in fields { - if field.contains("bindgen") { - // Similar to struct names, just skip these ones - continue; - } - - // HACK: This will break if some struct actually has a field called `type_` - let c_field = if field == "type_" { "type" } else { field }; - - write!( - file, - r#" - assert_eq!( - std::mem::offset_of!({strukt}, {field}), - cpp!(unsafe [] -> usize as "size_t" {{ return offsetof({strukt}, {c_field}); }}), - ); - assert_eq!( - align_of_field!({strukt}, {field}), - cpp!(unsafe [] -> usize as "size_t" {{ return alignof({strukt}::{c_field}); }}), - ); -"#, - )?; - - if let ("romfs_dir", "name") | ("romfs_file", "name") | ("sockaddr", "sa_data") = - (strukt.as_str(), field.as_str()) - { - // These are variable length arrays, so we can't use sizeof() - continue; - } - - write!( - file, - r#" - assert_eq!( - size_of_field!({strukt}, {field}), - cpp!(unsafe [] -> usize as "size_t" {{ return sizeof({strukt}::{c_field}); }}), - ); -"#, - )?; - } - - writeln!(file, "}}")?; - } - - // TODO: we could probably rustfmt here - - Ok(output_file) - } -} diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout.rs similarity index 60% rename from ctru-sys/tests/layout_test.rs rename to ctru-sys/tests/layout.rs index 2d7aa186..735b4522 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout.rs @@ -12,35 +12,12 @@ extern crate shim_3ds; +use binding_helpers::{align_of, size_of}; + // TODO: might want to move this into a test crate so we can avoid compiling it // for non-test builds? Idk if there's a reasonable way to do it though. -fn size_of_ret(_f: F) -> usize -where - F: FnOnce(T) -> U, -{ - std::mem::size_of::() -} - -macro_rules! size_of_field { - ($ty:ty, $field:ident) => {{ - #[allow(unused_unsafe)] - size_of_ret(|t: $ty| unsafe { t.$field }) - }}; -} - -fn align_of_ret(_f: F) -> usize -where - F: FnOnce(T) -> U, -{ - std::mem::align_of::() -} - -macro_rules! align_of_field { - ($ty:ty, $field:ident) => {{ - #[allow(unused_unsafe)] - align_of_ret(|t: $ty| unsafe { t.$field }) - }}; -} +use cpp::cpp; +use std::mem; include!(concat!(env!("OUT_DIR"), "/layout_test.rs")); From 528618d8da5d48624dbbe36c51e71adf7145b3f7 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 27 Apr 2024 13:46:15 -0400 Subject: [PATCH 04/13] Big refactor again and get everything building --- binding-helpers/Cargo.toml | 7 +- binding-helpers/src/gen.rs | 277 ++++++++++++------- binding-helpers/src/lib.rs | 53 ++-- ctru-sys/Cargo.toml | 11 +- ctru-sys/build.rs | 25 +- ctru-sys/tests/{layout.rs => layout_test.rs} | 10 +- 6 files changed, 234 insertions(+), 149 deletions(-) rename ctru-sys/tests/{layout.rs => layout_test.rs} (67%) diff --git a/binding-helpers/Cargo.toml b/binding-helpers/Cargo.toml index 031eeefc..5ebbfd3d 100644 --- a/binding-helpers/Cargo.toml +++ b/binding-helpers/Cargo.toml @@ -8,7 +8,8 @@ edition = "2021" bindgen = "*" proc-macro2 = "1.0.81" quote = "1.0.36" -rust-format = { version = "0.3.4", features = ["token_stream"] } +regex = "1.10.4" +rust-format = { optional = true, version = "0.3.4", features = ["token_stream"] } -# [build-dependencies]cpp = "0.5.9" -# cpp_build = "0.5.9" +[features] +rustfmt = ["dep:rust-format"] diff --git a/binding-helpers/src/gen.rs b/binding-helpers/src/gen.rs index 7c9d3763..a4985dca 100644 --- a/binding-helpers/src/gen.rs +++ b/binding-helpers/src/gen.rs @@ -1,128 +1,225 @@ use std::cell::RefCell; use std::collections::{HashMap, HashSet}; -use std::fmt::Display; -use std::io; +use std::fs::File; +use std::io::Write; use std::path::Path; use std::rc::Rc; -use bindgen::callbacks::{DeriveInfo, FieldInfo, ParseCallbacks}; +use bindgen::callbacks::{ + DeriveInfo, DeriveTrait, FieldInfo, ImplementsTrait, ParseCallbacks, TypeKind, +}; use bindgen::FieldVisibilityKind; use proc_macro2::TokenStream; -use quote::{format_ident, quote}; -use rust_format::{Formatter, RustFmt}; - -#[derive(Default, Debug)] -struct StructInfo { - fields: HashMap>, - names: HashSet, -} +use quote::{format_ident, quote, TokenStreamExt}; -#[derive(Default, Debug)] -pub struct LayoutTests(Rc>); +use regex::Regex; +#[cfg(feature = "rustfmt")] +use rust_format::{Formatter, RustFmt}; #[derive(Debug)] -pub enum Error { - Io(io::Error), - Format(rust_format::Error), -} +pub struct LayoutTestCallbacks(Rc); -impl Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Error::Io(io) => write!(f, "IO error: {io}"), - Error::Format(fmt) => write!(f, "Format error: {fmt}"), - } +impl LayoutTestCallbacks { + pub fn new() -> (Self, Rc) { + let generator = Rc::new(LayoutTestGenerator::new()); + (Self(Rc::clone(&generator)), generator) } } -impl From for Error { - fn from(e: io::Error) -> Self { - Self::Io(e) +impl ParseCallbacks for LayoutTestCallbacks { + fn header_file(&self, filename: &str) { + self.0.headers.borrow_mut().insert(filename.to_string()); } -} -impl From for Error { - fn from(e: rust_format::Error) -> Self { - Self::Format(e) + fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { + match info.kind { + TypeKind::Struct | TypeKind::Enum => { + self.0 + .fields + .borrow_mut() + .insert(info.name.to_string(), HashSet::new()); + } + TypeKind::Union => { + // layout tests don't handle unions for now, just skip it + println!( + "cargo:warning=Skipping generated tests for union {}", + info.name, + ); + self.0.blocklist_type(info.name); + } + } + + Vec::new() } -} -impl LayoutTests { - pub fn new() -> (Self, Self) { - let this = Self::default(); + fn blocklisted_type_implements_trait( + &self, + name: &str, + _derive_trait: DeriveTrait, + ) -> Option { + self.0.blocklist_type(name); + None + } + + fn field_visibility(&self, info: FieldInfo<'_>) -> Option { + self.0 + .fields + .borrow_mut() + .entry(info.type_name.to_string()) + .or_default() + .insert(info.field_name.to_string()); - (Self(Rc::clone(&this.0)), this) + None } +} - pub fn generate_layout_tests(&self, output_path: impl AsRef) -> Result<(), Error> { - let mut output = Vec::new(); +#[derive(Debug)] +pub struct LayoutTestGenerator { + fields: RefCell>>, + blocklist: RefCell>, + headers: RefCell>, +} - for (struct_name, fields) in &self.0.borrow().fields { - if struct_name.contains("bindgen") { - continue; - } - output.push(struct_layout_test(struct_name, fields)); +impl LayoutTestGenerator { + fn new() -> Self { + Self { + fields: Default::default(), + blocklist: Default::default(), + headers: Default::default(), } + } - let tokens = quote! { #(#output)* }; + pub fn blocklist_type(&self, pattern: &str) -> &Self { + self.blocklist + .borrow_mut() + .push(Regex::new(pattern).unwrap()); + self + } - // We can't use quote! here because it will reformat <> includes badly - // and also doesn't seem to play nice with whatever cpp! expands to. - let header = " - use cpp::cpp; + pub fn generate_layout_tests(&self, output_path: impl AsRef) -> Result<(), crate::Error> { + let mut file = File::create(output_path)?; - cpp! {{ - #include - #include <3ds.h> - }} - "; + // Since quote! tokenizes its input, it would result in invalid C++ for + // the `#include` directives (no whitespace/newlines), so we basically + // need to drop in the include headers here "manually" by writing them + // into the cpp! macro invocation. + file.write_all(b"cpp! {{\n")?; + for included_file in self.headers.borrow().iter() { + writeln!(file, " #include \"{included_file}\"")?; + } + file.write_all(b"}}\n\n")?; + + let test_tokens = self.build_tests(); - std::fs::write(&output_path, String::from(header) + &tokens.to_string())?; - RustFmt::default().format_file(output_path)?; + file.write_all( + #[cfg(feature = "rustfmt")] + RustFmt::default().format_tokens(test_tokens)?.as_bytes(), + #[cfg(not(feature = "rustfmt"))] + test_tokens.to_string().as_bytes(), + )?; Ok(()) } -} - -fn struct_layout_test( - struct_name: &str, - field_names: &HashSet, -) -> proc_macro2::TokenStream { - let name = format_ident!("{struct_name}"); - let test_name = format_ident!("layout_test_{struct_name}"); + fn build_tests(&self) -> TokenStream { + let mut output = TokenStream::new(); - let mut field_tests = Vec::new(); - field_tests.push(assert_eq(quote!(size_of!(#name)), quote!(sizeof(#name)))); - field_tests.push(assert_eq(quote!(align_of!(#name)), quote!(alignof(#name)))); + output.append_all(build_preamble()); - for field in field_names { - if field.contains("bindgen") { - continue; + 'structs: for struct_name in self.fields.borrow().keys() { + for pattern in self.blocklist.borrow().iter() { + if pattern.is_match(struct_name) { + continue 'structs; + } + } + output.append_all(self.build_struct_test(struct_name)); } - let field = if field == "type_" { "type" } else { field }; + output + } + + fn build_struct_test(&self, struct_name: &str) -> proc_macro2::TokenStream { + let name = format_ident!("{struct_name}"); - let field = format_ident!("{field}"); - field_tests.push(assert_eq( - quote!(size_of!(#name::#field)), - quote!(sizeof(#name::#field)), + let test_name = format_ident!("layout_test_{struct_name}"); + + let mut field_tests = Vec::new(); + field_tests.push(build_assert_eq( + quote!(size_of!(#name)), + quote!(sizeof(#name)), )); - field_tests.push(assert_eq( - quote!(align_of!(#name::#field)), - quote!(alignof(#name::#field)), + field_tests.push(build_assert_eq( + quote!(align_of!(#name)), + quote!(alignof(#name)), )); + + for field in self.fields.borrow().get(struct_name).into_iter().flatten() { + let field = format_ident!("{field}"); + + field_tests.push(build_assert_eq( + quote!(size_of!(#name::#field)), + quote!(sizeof(#name::#field)), + )); + + field_tests.push(build_assert_eq( + quote!(align_of!(#name::#field)), + quote!(alignof(#name::#field)), + )); + + field_tests.push(build_assert_eq( + quote!(offset_of!(#name, #field)), + quote!(offsetof(#name, #field)), + )); + } + + quote! { + #[test] + fn #test_name() { + #(#field_tests);* + } + } } +} +fn build_preamble() -> TokenStream { quote! { - #[test] - fn #test_name() { - #(#field_tests);* + use cpp::cpp; + + macro_rules! size_of { + ($ty:ident::$field:ident) => {{ + size_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::size_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::size_of_val(&$expr) + }; + } + + macro_rules! align_of { + ($ty:ident::$field:ident) => {{ + align_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::align_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::align_of_val(&$expr) + }; + } + + fn size_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::size_of::() + } + + fn align_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::align_of::() } } } -fn assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { +fn build_assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { quote! { assert_eq!( #rust_lhs, @@ -133,23 +230,3 @@ fn assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { ); } } - -impl ParseCallbacks for LayoutTests { - fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { - self.0.borrow_mut().names.insert(info.name.to_string()); - Vec::new() - } - - // We don't actually ever change visibility, but this allows us to keep track - // of all the fields in the structs bindgen processes. - fn field_visibility(&self, info: FieldInfo<'_>) -> Option { - self.0 - .borrow_mut() - .fields - .entry(info.type_name.to_string()) - .or_default() - .insert(info.field_name.to_string()); - - None - } -} diff --git a/binding-helpers/src/lib.rs b/binding-helpers/src/lib.rs index 1bd61038..190ec7fe 100644 --- a/binding-helpers/src/lib.rs +++ b/binding-helpers/src/lib.rs @@ -1,37 +1,34 @@ pub mod gen; -#[macro_export] -macro_rules! size_of { - ($ty:ident::$field:ident) => {{ - $crate::size_of_ret(|x: $ty| x.$field) - }}; - ($ty:ty) => { - ::std::mem::size_of::<$ty>() - }; - ($expr:expr) => { - ::std::mem::size_of_val(&$expr) - }; +use std::{fmt, io}; + +#[derive(Debug)] +#[non_exhaustive] +pub enum Error { + Io(io::Error), + #[cfg(feature = "rustfmt")] + Format(rust_format::Error), } -#[macro_export] -macro_rules! align_of { - ($ty:ident::$field:ident) => {{ - $create::align_of_ret(|x: $ty| x.$field) - }}; - ($ty:ty) => { - ::std::mem::align_of::<$ty>() - }; - ($expr:expr) => { - ::std::mem::align_of_val(&$expr) - }; +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Error::Io(io) => write!(f, "I/O error: {io}"), + #[cfg(feature = "rustfmt")] + Error::Format(fmt) => write!(f, "Format error: {fmt}"), + } + } } -#[doc(hidden)] -pub fn size_of_ret(_f: impl Fn(U) -> T) -> usize { - ::std::mem::size_of::() +impl From for Error { + fn from(e: io::Error) -> Self { + Self::Io(e) + } } -#[doc(hidden)] -pub fn align_of_ret(_f: impl Fn(U) -> T) -> usize { - ::std::mem::align_of::() +#[cfg(feature = "rustfmt")] +impl From for Error { + fn from(e: rust_format::Error) -> Self { + Self::Format(e) + } } diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index b0e8527e..ee42d33b 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -13,10 +13,14 @@ links = "ctru" edition = "2021" [features] +default = [] + +## Enables generating C++/Rust layout comparison tests. +## Downstream users of `ctru-sys` shouldn't need to use this feature. layout-tests = [] [[test]] -path = "tests/layout.rs" +path = "tests/layout_test.rs" name = "layout" required-features = ["layout-tests"] @@ -24,17 +28,18 @@ required-features = ["layout-tests"] libc = { version = "0.2.121", default-features = false } [build-dependencies] -bindgen = { version = "0.66.1", features = ["experimental"] } +bindgen = { version = "0.69", features = ["experimental"] } cc = "1.0" cpp_build = "0.5.9" doxygen-rs = "0.4.2" itertools = "0.11.0" which = "4.4.0" -binding-helpers = { git = "https://github.com/rust3ds/ctru-rs.git" } +binding-helpers = { git = "https://github.com/rust3ds/ctru-rs.git", features = ["rustfmt"] } [dev-dependencies] shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } +cpp = "0.5.9" [package.metadata.docs.rs] default-target = "armv6k-nintendo-3ds" diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 492c750e..bf36333c 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -1,14 +1,12 @@ use bindgen::callbacks::ParseCallbacks; use bindgen::{Builder, RustTarget}; -use binding_helpers::gen::LayoutTests; +use binding_helpers::gen::LayoutTestCallbacks; use itertools::Itertools; -use std::io::{self, Write}; use std::env; use std::error::Error; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; -use std::rc::Rc; #[derive(Debug)] struct CustomCallbacks; @@ -108,7 +106,7 @@ fn main() { .flag("-fshort-enums") .get_compiler(); - let (test_gen_callbacks, test_generator) = LayoutTests::new(); + let (test_callbacks, test_generator) = LayoutTestCallbacks::new(); // Build libctru bindings let bindings = Builder::default() @@ -139,7 +137,7 @@ fn main() { .wrap_static_fns_path(out_dir.join("libctru_statics_wrapper")) .clang_args(clang.args().iter().map(|s| s.to_str().unwrap())) .parse_callbacks(Box::new(CustomCallbacks)) - .parse_callbacks(Box::new(test_gen_callbacks)) + .parse_callbacks(Box::new(test_callbacks)) .generate() .expect("unable to generate bindings"); @@ -152,10 +150,23 @@ fn main() { .compile("ctru_statics_wrapper"); if env::var("CARGO_FEATURE_LAYOUT_TESTS").is_ok() { - let test_file = out_dir.join("layout_tests.rs"); + let test_file = out_dir.join("generated_layout_test.rs"); test_generator + // We can't figure out it's an opaque type just from callbacks, + // so explicitly blocklist generated tests for MiiData: + .blocklist_type("MiiData.*") + // There are several other bindgen-generated types that we don't + // want/need to check as well: + .blocklist_type("tag_CMAP.*") + .blocklist_type("sig(event|info_t)") + .blocklist_type("bintime") + .blocklist_type("Thread_tag") + .blocklist_type("aptCaptureBufInfo.*") + .blocklist_type("fontGlyphPos_s.*") + .blocklist_type("__.*_t") + .blocklist_type("fd_set") .generate_layout_tests(&test_file) - .expect("Couldn't write layout tests!"); + .unwrap_or_else(|err| panic!("Failed to generate layout tests: {err}")); cpp_build::Config::default() .compiler(cpp) diff --git a/ctru-sys/tests/layout.rs b/ctru-sys/tests/layout_test.rs similarity index 67% rename from ctru-sys/tests/layout.rs rename to ctru-sys/tests/layout_test.rs index 735b4522..9cd53c65 100644 --- a/ctru-sys/tests/layout.rs +++ b/ctru-sys/tests/layout_test.rs @@ -12,12 +12,6 @@ extern crate shim_3ds; -use binding_helpers::{align_of, size_of}; +use ctru_sys::*; -// TODO: might want to move this into a test crate so we can avoid compiling it -// for non-test builds? Idk if there's a reasonable way to do it though. - -use cpp::cpp; -use std::mem; - -include!(concat!(env!("OUT_DIR"), "/layout_test.rs")); +include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); From 4c84d455185a969573a3afd70cd87138073a8224 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 3 May 2024 18:25:33 -0400 Subject: [PATCH 05/13] Fix up merge conflicts and share some dependencies Workspace Cargo.toml can define deps that get inherited by crates, which in our case is particularly useful for bindgen but also means we can update less places if our git dependencies change names etc. --- Cargo.toml | 22 +++++++------- binding-helpers/Cargo.toml | 2 +- ctru-rs/Cargo.toml | 6 ++-- ctru-sys/Cargo.toml | 14 ++++----- ctru-sys/build.rs | 62 ++++++++++++++++++++------------------ 5 files changed, 54 insertions(+), 52 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index af85a6a5..be4b8490 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,23 +1,23 @@ [workspace] -<<<<<<< HEAD -members = ["ctru-rs", "ctru-sys", "test-runner"] -======= -members = ["binding-helpers", "ctru-rs", "ctru-sys"] ->>>>>>> 2c06946 (Big refactor to use quote and separate crate) +members = ["binding-helpers", "ctru-rs", "ctru-sys", "test-runner"] default-members = ["ctru-rs", "ctru-sys"] resolver = "2" +[workspace.dependencies] +bindgen = "0.69" +libc = { version = "0.2.121", default-features = false } +shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } +pthread-3ds = { git = "https://github.com/rust3ds/pthread-3ds.git" } +test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } + [patch.'https://github.com/rust3ds/ctru-rs.git'] # Make sure all dependencies use the local packages. This is needed for things # like pthread-3ds that rely on ctru-sys, and test-runner which relies on ctru-rs +test-runner = { path = "test-runner" } ctru-rs = { path = "ctru-rs" } ctru-sys = { path = "ctru-sys" } -<<<<<<< HEAD -test-runner = { path = "test-runner" } +binding-helpers = { path = "binding-helpers" } # This was the previous git repo for test-runner -[patch."https://github.com/rust3ds/test-runner"] +[patch."https://github.com/rust3ds/test-runner.git"] test-runner = { path = "test-runner" } -======= -binding-helpers = { path = "binding-helpers" } ->>>>>>> 2c06946 (Big refactor to use quote and separate crate) diff --git a/binding-helpers/Cargo.toml b/binding-helpers/Cargo.toml index 5ebbfd3d..19fe0b2a 100644 --- a/binding-helpers/Cargo.toml +++ b/binding-helpers/Cargo.toml @@ -5,7 +5,7 @@ version = "0.1.0" edition = "2021" [dependencies] -bindgen = "*" +bindgen = { workspace = true } proc-macro2 = "1.0.81" quote = "1.0.36" regex = "1.10.4" diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index 38c9bc4e..4e5883f9 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -20,9 +20,9 @@ name = "ctru" cfg-if = "1.0" ctru-sys = { path = "../ctru-sys", version = "0.5.0" } const-zero = "0.1.0" -shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } -pthread-3ds = { git = "https://github.com/rust3ds/pthread-3ds.git" } -libc = "0.2.121" +shim-3ds = { workspace = true } +pthread-3ds = { workspace = true } +libc = { workspace = true } bitflags = "2.3.3" macaddr = "1.0.1" widestring = "1.0.2" diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index ee42d33b..121be14d 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -17,7 +17,7 @@ default = [] ## Enables generating C++/Rust layout comparison tests. ## Downstream users of `ctru-sys` shouldn't need to use this feature. -layout-tests = [] +layout-tests = ["dep:cpp_build"] [[test]] path = "tests/layout_test.rs" @@ -25,21 +25,21 @@ name = "layout" required-features = ["layout-tests"] [dependencies] -libc = { version = "0.2.121", default-features = false } +libc = { workspace = true } [build-dependencies] -bindgen = { version = "0.69", features = ["experimental"] } +bindgen = { workspace = true, features = ["experimental"] } cc = "1.0" -cpp_build = "0.5.9" +cpp_build = { optional = true, git = "https://github.com/mystor/rust-cpp.git" } doxygen-rs = "0.4.2" itertools = "0.11.0" which = "4.4.0" binding-helpers = { git = "https://github.com/rust3ds/ctru-rs.git", features = ["rustfmt"] } [dev-dependencies] -shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } -test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } -cpp = "0.5.9" +shim-3ds = { workspace = true } +test-runner = { workspace = true } +cpp = { git = "https://github.com/mystor/rust-cpp.git" } [package.metadata.docs.rs] default-target = "armv6k-nintendo-3ds" diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index bf36333c..a15d92d5 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -1,8 +1,10 @@ use bindgen::callbacks::ParseCallbacks; use bindgen::{Builder, RustTarget}; -use binding_helpers::gen::LayoutTestCallbacks; use itertools::Itertools; +#[cfg(feature = "layout-tests")] +use binding_helpers::gen::LayoutTestCallbacks; + use std::env; use std::error::Error; use std::path::{Path, PathBuf}; @@ -54,25 +56,28 @@ fn main() { detect_and_track_libctru(); - let gcc_version = get_gcc_version(PathBuf::from(&devkitarm).join("bin/arm-none-eabi-gcc")); + let bin_dir = Path::new(devkitarm.as_str()).join("bin"); + let cc = bin_dir.join("arm-none-eabi-gcc"); + let ar = bin_dir.join("arm-none-eabi-ar"); + + #[cfg(feature = "layout-tests")] + let cpp = bin_dir.join("arm-none-eabi-g++"); - let include_path = PathBuf::from_iter([devkitpro.as_str(), "libctru", "include"]); + let include_path = Path::new(devkitpro.as_str()).join("libctru/include"); let ctru_header = include_path.join("3ds.h"); let sysroot = Path::new(&devkitarm).join("arm-none-eabi"); let system_include = sysroot.join("include"); - let gcc_include = PathBuf::from(format!( - "{devkitarm}/lib/gcc/arm-none-eabi/{gcc_version}/include" - )); let errno_header = system_include.join("errno.h"); - // Compile static inline fns wrapper - let cc = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-gcc"); - let cpp = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-g++"); - let ar = Path::new(devkitarm.as_str()).join("bin/arm-none-eabi-ar"); + let gcc_version = get_gcc_version(&cc); + let gcc_include = Path::new(&devkitarm) + .join("lib/gcc/arm-none-eabi") + .join(gcc_version) + .join("include"); - let mut builder = cc::Build::new(); - builder + let mut cc_build = cc::Build::new(); + cc_build .compiler(cc) .archiver(ar) .include(&include_path) @@ -85,7 +90,7 @@ fn main() { .flag("-mtp=soft") .flag("-Wno-deprecated-declarations"); - let clang = builder + let clang = cc_build .clone() .compiler("clang") // bindgen uses clang, so we need to tell it where devkitARM sysroot / libs are: @@ -106,10 +111,11 @@ fn main() { .flag("-fshort-enums") .get_compiler(); + #[cfg(feature = "layout-tests")] let (test_callbacks, test_generator) = LayoutTestCallbacks::new(); // Build libctru bindings - let bindings = Builder::default() + let binding_builder = Builder::default() .header(ctru_header.to_str().unwrap()) .header(errno_header.to_str().unwrap()) .rust_target(RustTarget::Nightly) @@ -136,20 +142,23 @@ fn main() { .wrap_static_fns(true) .wrap_static_fns_path(out_dir.join("libctru_statics_wrapper")) .clang_args(clang.args().iter().map(|s| s.to_str().unwrap())) - .parse_callbacks(Box::new(CustomCallbacks)) - .parse_callbacks(Box::new(test_callbacks)) - .generate() - .expect("unable to generate bindings"); + .parse_callbacks(Box::new(CustomCallbacks)); + + #[cfg(feature = "layout-tests")] + let binding_builder = binding_builder.parse_callbacks(Box::new(test_callbacks)); - bindings + binding_builder + .generate() + .expect("unable to generate bindings") .write_to_file(out_dir.join("bindings.rs")) .expect("Couldn't write bindings!"); - builder + cc_build .file(out_dir.join("libctru_statics_wrapper.c")) .compile("ctru_statics_wrapper"); - if env::var("CARGO_FEATURE_LAYOUT_TESTS").is_ok() { + #[cfg(feature = "layout-tests")] + { let test_file = out_dir.join("generated_layout_test.rs"); test_generator // We can't figure out it's an opaque type just from callbacks, @@ -168,20 +177,13 @@ fn main() { .generate_layout_tests(&test_file) .unwrap_or_else(|err| panic!("Failed to generate layout tests: {err}")); - cpp_build::Config::default() + cpp_build::Config::from(cc_build) .compiler(cpp) - .include(include_path) - .flag("-march=armv6k") - .flag("-mtune=mpcore") - .flag("-mfloat-abi=hard") - .flag("-mfpu=vfp") - .flag("-mtp=soft") - .flag("-Wno-deprecated-declarations") .build(test_file); } } -fn get_gcc_version(path_to_gcc: PathBuf) -> String { +fn get_gcc_version(path_to_gcc: &Path) -> String { let Output { stdout, .. } = Command::new(path_to_gcc) .arg("--version") .stderr(Stdio::inherit()) From b0631efc25ee8921b26eb22a722afd0d2af64abd Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 4 May 2024 15:52:21 -0400 Subject: [PATCH 06/13] Simplify test gen with a submod instead of crate --- Cargo.toml | 4 +-- binding-helpers/Cargo.toml | 15 -------- binding-helpers/src/lib.rs | 34 ------------------- ctru-sys/Cargo.toml | 17 +++++++--- ctru-sys/build.rs | 17 ++++++---- .../src/gen.rs => ctru-sys/build/test_gen.rs | 22 ++++++------ 6 files changed, 37 insertions(+), 72 deletions(-) delete mode 100644 binding-helpers/Cargo.toml delete mode 100644 binding-helpers/src/lib.rs rename binding-helpers/src/gen.rs => ctru-sys/build/test_gen.rs (91%) diff --git a/Cargo.toml b/Cargo.toml index be4b8490..29d38753 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,10 +1,9 @@ [workspace] -members = ["binding-helpers", "ctru-rs", "ctru-sys", "test-runner"] +members = ["ctru-rs", "ctru-sys", "test-runner"] default-members = ["ctru-rs", "ctru-sys"] resolver = "2" [workspace.dependencies] -bindgen = "0.69" libc = { version = "0.2.121", default-features = false } shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } pthread-3ds = { git = "https://github.com/rust3ds/pthread-3ds.git" } @@ -16,7 +15,6 @@ test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } test-runner = { path = "test-runner" } ctru-rs = { path = "ctru-rs" } ctru-sys = { path = "ctru-sys" } -binding-helpers = { path = "binding-helpers" } # This was the previous git repo for test-runner [patch."https://github.com/rust3ds/test-runner.git"] diff --git a/binding-helpers/Cargo.toml b/binding-helpers/Cargo.toml deleted file mode 100644 index 19fe0b2a..00000000 --- a/binding-helpers/Cargo.toml +++ /dev/null @@ -1,15 +0,0 @@ -[package] -name = "binding-helpers" -description = "A helper crate for testing Rust bindings' definitions against their C/C++ counterparts." -version = "0.1.0" -edition = "2021" - -[dependencies] -bindgen = { workspace = true } -proc-macro2 = "1.0.81" -quote = "1.0.36" -regex = "1.10.4" -rust-format = { optional = true, version = "0.3.4", features = ["token_stream"] } - -[features] -rustfmt = ["dep:rust-format"] diff --git a/binding-helpers/src/lib.rs b/binding-helpers/src/lib.rs deleted file mode 100644 index 190ec7fe..00000000 --- a/binding-helpers/src/lib.rs +++ /dev/null @@ -1,34 +0,0 @@ -pub mod gen; - -use std::{fmt, io}; - -#[derive(Debug)] -#[non_exhaustive] -pub enum Error { - Io(io::Error), - #[cfg(feature = "rustfmt")] - Format(rust_format::Error), -} - -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Error::Io(io) => write!(f, "I/O error: {io}"), - #[cfg(feature = "rustfmt")] - Error::Format(fmt) => write!(f, "Format error: {fmt}"), - } - } -} - -impl From for Error { - fn from(e: io::Error) -> Self { - Self::Io(e) - } -} - -#[cfg(feature = "rustfmt")] -impl From for Error { - fn from(e: rust_format::Error) -> Self { - Self::Format(e) - } -} diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index 121be14d..af93052f 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -17,24 +17,33 @@ default = [] ## Enables generating C++/Rust layout comparison tests. ## Downstream users of `ctru-sys` shouldn't need to use this feature. -layout-tests = ["dep:cpp_build"] +layout-tests = [ + "dep:cpp_build", + "dep:proc-macro2", + "dep:quote", + "dep:regex", + "dep:rust-format", +] [[test]] path = "tests/layout_test.rs" -name = "layout" +name = "layout_test" required-features = ["layout-tests"] [dependencies] libc = { workspace = true } [build-dependencies] -bindgen = { workspace = true, features = ["experimental"] } +bindgen = { version = "0.69", features = ["experimental"] } cc = "1.0" cpp_build = { optional = true, git = "https://github.com/mystor/rust-cpp.git" } doxygen-rs = "0.4.2" itertools = "0.11.0" +proc-macro2 = { version = "1.0.81", optional = true } +quote = { version = "1.0.36", optional = true } +regex = { version = "1.10.4", optional = true } +rust-format = { version = "0.3.4", optional = true, features = ["token_stream"] } which = "4.4.0" -binding-helpers = { git = "https://github.com/rust3ds/ctru-rs.git", features = ["rustfmt"] } [dev-dependencies] shim-3ds = { workspace = true } diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index a15d92d5..5c43a893 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -2,14 +2,20 @@ use bindgen::callbacks::ParseCallbacks; use bindgen::{Builder, RustTarget}; use itertools::Itertools; -#[cfg(feature = "layout-tests")] -use binding_helpers::gen::LayoutTestCallbacks; - use std::env; use std::error::Error; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; +// This allows us to have a directory layout of build/*.rs which is a little +// cleaner than having all the submodules as siblings to build.rs. +mod build { + #[cfg(feature = "layout-tests")] + pub mod test_gen; +} + +use build::*; + #[derive(Debug)] struct CustomCallbacks; @@ -111,9 +117,6 @@ fn main() { .flag("-fshort-enums") .get_compiler(); - #[cfg(feature = "layout-tests")] - let (test_callbacks, test_generator) = LayoutTestCallbacks::new(); - // Build libctru bindings let binding_builder = Builder::default() .header(ctru_header.to_str().unwrap()) @@ -144,6 +147,8 @@ fn main() { .clang_args(clang.args().iter().map(|s| s.to_str().unwrap())) .parse_callbacks(Box::new(CustomCallbacks)); + #[cfg(feature = "layout-tests")] + let (test_callbacks, test_generator) = test_gen::LayoutTestCallbacks::new(); #[cfg(feature = "layout-tests")] let binding_builder = binding_builder.parse_callbacks(Box::new(test_callbacks)); diff --git a/binding-helpers/src/gen.rs b/ctru-sys/build/test_gen.rs similarity index 91% rename from binding-helpers/src/gen.rs rename to ctru-sys/build/test_gen.rs index a4985dca..19ed3206 100644 --- a/binding-helpers/src/gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -1,5 +1,11 @@ +//! This module contains the code to generate layout tests comparing generated +//! Rust bindings to the actual C types defined in libctru. We use [`cpp_build`] +//! to compile helper functions that return the real `sizeof`/`alignof` those types +//! and compare them to the ones generated by `bindgen`. + use std::cell::RefCell; use std::collections::{HashMap, HashSet}; +use std::error::Error; use std::fs::File; use std::io::Write; use std::path::Path; @@ -13,7 +19,6 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote, TokenStreamExt}; use regex::Regex; -#[cfg(feature = "rustfmt")] use rust_format::{Formatter, RustFmt}; #[derive(Debug)] @@ -96,7 +101,10 @@ impl LayoutTestGenerator { self } - pub fn generate_layout_tests(&self, output_path: impl AsRef) -> Result<(), crate::Error> { + pub fn generate_layout_tests( + &self, + output_path: impl AsRef, + ) -> Result<(), Box> { let mut file = File::create(output_path)?; // Since quote! tokenizes its input, it would result in invalid C++ for @@ -111,12 +119,7 @@ impl LayoutTestGenerator { let test_tokens = self.build_tests(); - file.write_all( - #[cfg(feature = "rustfmt")] - RustFmt::default().format_tokens(test_tokens)?.as_bytes(), - #[cfg(not(feature = "rustfmt"))] - test_tokens.to_string().as_bytes(), - )?; + file.write_all(RustFmt::default().format_tokens(test_tokens)?.as_bytes())?; Ok(()) } @@ -140,8 +143,7 @@ impl LayoutTestGenerator { fn build_struct_test(&self, struct_name: &str) -> proc_macro2::TokenStream { let name = format_ident!("{struct_name}"); - - let test_name = format_ident!("layout_test_{struct_name}"); + let test_name = format_ident!("layout_test_{name}"); let mut field_tests = Vec::new(); field_tests.push(build_assert_eq( From 9469b7e64dd132b4ee95e48745571c7fe128a940 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 4 May 2024 15:56:07 -0400 Subject: [PATCH 07/13] Move preamble into stub file instead of generating --- ctru-sys/build/test_gen.rs | 40 ----------------------------------- ctru-sys/tests/layout_test.rs | 33 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 40 deletions(-) diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index 19ed3206..898e62d9 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -127,8 +127,6 @@ impl LayoutTestGenerator { fn build_tests(&self) -> TokenStream { let mut output = TokenStream::new(); - output.append_all(build_preamble()); - 'structs: for struct_name in self.fields.borrow().keys() { for pattern in self.blocklist.borrow().iter() { if pattern.is_match(struct_name) { @@ -183,44 +181,6 @@ impl LayoutTestGenerator { } } -fn build_preamble() -> TokenStream { - quote! { - use cpp::cpp; - - macro_rules! size_of { - ($ty:ident::$field:ident) => {{ - size_of_ret(|x: $ty| x.$field) - }}; - ($ty:ty) => { - ::std::mem::size_of::<$ty>() - }; - ($expr:expr) => { - ::std::mem::size_of_val(&$expr) - }; - } - - macro_rules! align_of { - ($ty:ident::$field:ident) => {{ - align_of_ret(|x: $ty| x.$field) - }}; - ($ty:ty) => { - ::std::mem::align_of::<$ty>() - }; - ($expr:expr) => { - ::std::mem::align_of_val(&$expr) - }; - } - - fn size_of_ret(_f: impl Fn(U) -> T) -> usize { - ::std::mem::size_of::() - } - - fn align_of_ret(_f: impl Fn(U) -> T) -> usize { - ::std::mem::align_of::() - } - } -} - fn build_assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { quote! { assert_eq!( diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs index 9cd53c65..a9ff59bf 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout_test.rs @@ -12,6 +12,39 @@ extern crate shim_3ds; +use cpp::cpp; use ctru_sys::*; +fn size_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::size_of::() +} + +macro_rules! size_of { + ($ty:ident::$field:ident) => {{ + size_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::size_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::size_of_val(&$expr) + }; +} + +fn align_of_ret(_f: impl Fn(U) -> T) -> usize { + ::std::mem::align_of::() +} + +macro_rules! align_of { + ($ty:ident::$field:ident) => {{ + align_of_ret(|x: $ty| x.$field) + }}; + ($ty:ty) => { + ::std::mem::align_of::<$ty>() + }; + ($expr:expr) => { + ::std::mem::align_of_val(&$expr) + }; +} + include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); From 0336a187c54ea7333c2d333bececed8189c97e9a Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 5 May 2024 19:57:13 -0400 Subject: [PATCH 08/13] Misc. cleanup and update CI to use all features --- .github/workflows/ci.yml | 4 +- ctru-sys/build.rs | 31 ++++----- ctru-sys/build/test_gen.rs | 125 ++++++++++++++++++++-------------- ctru-sys/src/lib.rs | 9 +-- ctru-sys/tests/layout_test.rs | 56 ++++++++++++++- 5 files changed, 149 insertions(+), 76 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2db5b645..fe58c0d1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,12 +80,12 @@ jobs: # unless cargo-3ds actually runs them as separate commands. See # https://github.com/rust3ds/cargo-3ds/issues/44 for more details - name: Build lib and integration tests - run: cargo 3ds test --no-run --tests + run: cargo 3ds test --no-run --tests --all-features - name: Run lib and integration tests uses: rust3ds/test-runner/run-tests@v1 with: - args: --tests + args: --tests --all-features - name: Build and run doc tests uses: rust3ds/test-runner/run-tests@v1 diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 5c43a893..db6f86f7 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -14,8 +14,6 @@ mod build { pub mod test_gen; } -use build::*; - #[derive(Debug)] struct CustomCallbacks; @@ -148,7 +146,7 @@ fn main() { .parse_callbacks(Box::new(CustomCallbacks)); #[cfg(feature = "layout-tests")] - let (test_callbacks, test_generator) = test_gen::LayoutTestCallbacks::new(); + let (test_callbacks, test_generator) = build::test_gen::LayoutTestCallbacks::new(); #[cfg(feature = "layout-tests")] let binding_builder = binding_builder.parse_callbacks(Box::new(test_callbacks)); @@ -166,24 +164,25 @@ fn main() { { let test_file = out_dir.join("generated_layout_test.rs"); test_generator - // We can't figure out it's an opaque type just from callbacks, - // so explicitly blocklist generated tests for MiiData: - .blocklist_type("MiiData.*") - // There are several other bindgen-generated types that we don't - // want/need to check as well: - .blocklist_type("tag_CMAP.*") - .blocklist_type("sig(event|info_t)") - .blocklist_type("bintime") + // There are several bindgen-generated types that we don't want/need to check + // (because they are opaque, use bitfields, anonymous structs etc.) .blocklist_type("Thread_tag") - .blocklist_type("aptCaptureBufInfo.*") - .blocklist_type("fontGlyphPos_s.*") - .blocklist_type("__.*_t") - .blocklist_type("fd_set") + .blocklist_type("MiiData.*") + .blocklist_type("ExHeader_(System|Arm11).*") + .blocklist_type("FS_((Ext|System)SaveData|Program)Info") + .blocklist_type("FS_ProgramInfo") + .blocklist_type("Y2RU_ConversionParams") + .blocklist_field("romfs_(dir|file)", "name") + // Bindgen generated types have no c++ equivalent: + .blocklist_type(".*__bindgen.*") + .blocklist_field(".*", "__bindgen.*") + // TODO: maybe we could translate type <-> `type_` or something... + .blocklist_field(".*", "type_") .generate_layout_tests(&test_file) .unwrap_or_else(|err| panic!("Failed to generate layout tests: {err}")); cpp_build::Config::from(cc_build) - .compiler(cpp) + .compiler(&cpp) .build(test_file); } } diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index 898e62d9..c2112d45 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -4,7 +4,7 @@ //! and compare them to the ones generated by `bindgen`. use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; use std::fs::File; use std::io::Write; @@ -33,25 +33,17 @@ impl LayoutTestCallbacks { impl ParseCallbacks for LayoutTestCallbacks { fn header_file(&self, filename: &str) { - self.0.headers.borrow_mut().insert(filename.to_string()); + self.0.headers.borrow_mut().push(filename.to_string()); } fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec { - match info.kind { - TypeKind::Struct | TypeKind::Enum => { - self.0 - .fields - .borrow_mut() - .insert(info.name.to_string(), HashSet::new()); - } - TypeKind::Union => { - // layout tests don't handle unions for now, just skip it - println!( - "cargo:warning=Skipping generated tests for union {}", - info.name, - ); - self.0.blocklist_type(info.name); - } + if let TypeKind::Union = info.kind { + // layout tests don't handle unions for now, just skip it + println!( + "cargo:warning=Skipping layout tests for union {}", + info.name, + ); + self.0.blocklist_type(info.name); } Vec::new() @@ -68,7 +60,7 @@ impl ParseCallbacks for LayoutTestCallbacks { fn field_visibility(&self, info: FieldInfo<'_>) -> Option { self.0 - .fields + .struct_fields .borrow_mut() .entry(info.type_name.to_string()) .or_default() @@ -80,27 +72,34 @@ impl ParseCallbacks for LayoutTestCallbacks { #[derive(Debug)] pub struct LayoutTestGenerator { - fields: RefCell>>, - blocklist: RefCell>, - headers: RefCell>, + struct_fields: RefCell>>, + blocklist: RefCell)>>, + headers: RefCell>, } impl LayoutTestGenerator { fn new() -> Self { Self { - fields: Default::default(), - blocklist: Default::default(), - headers: Default::default(), + struct_fields: RefCell::default(), + blocklist: RefCell::default(), + headers: RefCell::default(), } } pub fn blocklist_type(&self, pattern: &str) -> &Self { self.blocklist .borrow_mut() - .push(Regex::new(pattern).unwrap()); + .push((Regex::new(pattern).unwrap(), None)); self } + pub fn blocklist_field(&self, struct_pattern: &str, field_pattern: &str) -> &Self { + self.blocklist.borrow_mut().push(( + Regex::new(struct_pattern).unwrap(), + Some(Regex::new(field_pattern).unwrap()), + )); + self + } pub fn generate_layout_tests( &self, output_path: impl AsRef, @@ -108,18 +107,17 @@ impl LayoutTestGenerator { let mut file = File::create(output_path)?; // Since quote! tokenizes its input, it would result in invalid C++ for - // the `#include` directives (no whitespace/newlines), so we basically - // need to drop in the include headers here "manually" by writing them - // into the cpp! macro invocation. - file.write_all(b"cpp! {{\n")?; + // the `#include` directives (they would be missing whitespace/newlines), + // so we basically need to drop in the include headers here "manually" by + // writing them into the cpp! macro invocation. + file.write_all("cpp! {{".as_bytes())?; for included_file in self.headers.borrow().iter() { writeln!(file, " #include \"{included_file}\"")?; } - file.write_all(b"}}\n\n")?; - - let test_tokens = self.build_tests(); + file.write_all("}}\n".as_bytes())?; - file.write_all(RustFmt::default().format_tokens(test_tokens)?.as_bytes())?; + let test_tokens = RustFmt::default().format_tokens(self.build_tests())?; + file.write_all(test_tokens.as_bytes())?; Ok(()) } @@ -127,12 +125,17 @@ impl LayoutTestGenerator { fn build_tests(&self) -> TokenStream { let mut output = TokenStream::new(); - 'structs: for struct_name in self.fields.borrow().keys() { - for pattern in self.blocklist.borrow().iter() { - if pattern.is_match(struct_name) { - continue 'structs; - } + for struct_name in self.struct_fields.borrow().keys() { + if self + .blocklist + .borrow() + .iter() + .any(|(pat, field)| field.is_none() && pat.is_match(struct_name)) + { + println!("cargo:warning=Skipping layout tests for {struct_name}",); + continue; } + output.append_all(self.build_struct_test(struct_name)); } @@ -153,23 +156,41 @@ impl LayoutTestGenerator { quote!(alignof(#name)), )); - for field in self.fields.borrow().get(struct_name).into_iter().flatten() { - let field = format_ident!("{field}"); + let struct_fields = self.struct_fields.borrow(); + if let Some(fields) = struct_fields.get(struct_name) { + for field in fields { + if self + .blocklist + .borrow() + .iter() + .any(|(struct_pat, field_pat)| match field_pat { + Some(field_pat) => { + struct_pat.is_match(struct_name) && field_pat.is_match(field) + } + None => false, + }) + { + println!("cargo:warning=Skipping layout tests for {struct_name}::{field}",); + continue; + } - field_tests.push(build_assert_eq( - quote!(size_of!(#name::#field)), - quote!(sizeof(#name::#field)), - )); + let field = format_ident!("{field}"); - field_tests.push(build_assert_eq( - quote!(align_of!(#name::#field)), - quote!(alignof(#name::#field)), - )); + field_tests.push(build_assert_eq( + quote!(size_of!(#name::#field)), + quote!(sizeof(#name::#field)), + )); - field_tests.push(build_assert_eq( - quote!(offset_of!(#name, #field)), - quote!(offsetof(#name, #field)), - )); + field_tests.push(build_assert_eq( + quote!(align_of!(#name::#field)), + quote!(alignof(#name::#field)), + )); + + field_tests.push(build_assert_eq( + quote!(offset_of!(#name, #field)), + quote!(offsetof(#name, #field)), + )); + } } quote! { diff --git a/ctru-sys/src/lib.rs b/ctru-sys/src/lib.rs index ed2127b7..4e8e82f3 100644 --- a/ctru-sys/src/lib.rs +++ b/ctru-sys/src/lib.rs @@ -14,6 +14,11 @@ )] #![doc(html_root_url = "https://rust3ds.github.io/ctru-rs/crates")] +// Prevent linking errors from the standard `test` library when running `cargo 3ds test --lib`. +// See https://github.com/rust-lang/rust-analyzer/issues/14167 for why we use `not(rust_analyzer)` +#[cfg(all(test, not(rust_analyzer)))] +extern crate shim_3ds; + pub mod result; pub use result::*; @@ -36,7 +41,3 @@ pub use bindings::*; pub unsafe fn errno() -> s32 { *__errno() } - -// Prevent linking errors from the standard `test` library when running `cargo 3ds test --lib`. -#[cfg(test)] -extern crate shim_3ds; diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs index a9ff59bf..88350fc0 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout_test.rs @@ -12,6 +12,8 @@ extern crate shim_3ds; +use std::mem::offset_of; + use cpp::cpp; use ctru_sys::*; @@ -21,7 +23,7 @@ fn size_of_ret(_f: impl Fn(U) -> T) -> usize { macro_rules! size_of { ($ty:ident::$field:ident) => {{ - size_of_ret(|x: $ty| x.$field) + $crate::size_of_ret(|x: $ty| x.$field) }}; ($ty:ty) => { ::std::mem::size_of::<$ty>() @@ -37,7 +39,10 @@ fn align_of_ret(_f: impl Fn(U) -> T) -> usize { macro_rules! align_of { ($ty:ident::$field:ident) => {{ - align_of_ret(|x: $ty| x.$field) + // This matches the semantics of C++ alignof when it is applied to a struct + // member. Packed structs may under-align fields, so we take the minimum + // of the align of the struct and the type of the field itself. + $crate::align_of_ret(|x: $ty| x.$field).min(align_of!($ty)) }}; ($ty:ty) => { ::std::mem::align_of::<$ty>() @@ -48,3 +53,50 @@ macro_rules! align_of { } include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); + +mod helper_test { + macro_rules! packed_struct { + ($name:ident, $size:literal) => { + #[repr(C, packed($size))] + struct $name { + a: u8, + b: u16, + c: u32, + d: u64, + } + }; + } + + packed_struct!(PackedStruct1, 1); + packed_struct!(PackedStruct2, 2); + packed_struct!(PackedStruct4, 4); + packed_struct!(PackedStruct8, 8); + + #[test] + fn align_of_matches_cpp() { + // Expected values are based on C++: https://godbolt.org/z/dPnP7nEse + assert_eq!(align_of!(PackedStruct1), 1); + assert_eq!(align_of!(PackedStruct1::a), 1); + assert_eq!(align_of!(PackedStruct1::b), 1); + assert_eq!(align_of!(PackedStruct1::c), 1); + assert_eq!(align_of!(PackedStruct1::d), 1); + + assert_eq!(align_of!(PackedStruct2), 2); + assert_eq!(align_of!(PackedStruct2::a), 1); + assert_eq!(align_of!(PackedStruct2::b), 2); + assert_eq!(align_of!(PackedStruct2::c), 2); + assert_eq!(align_of!(PackedStruct2::d), 2); + + assert_eq!(align_of!(PackedStruct4), 4); + assert_eq!(align_of!(PackedStruct4::a), 1); + assert_eq!(align_of!(PackedStruct4::b), 2); + assert_eq!(align_of!(PackedStruct4::c), 4); + assert_eq!(align_of!(PackedStruct4::d), 4); + + assert_eq!(align_of!(PackedStruct8), 8); + assert_eq!(align_of!(PackedStruct8::a), 1); + assert_eq!(align_of!(PackedStruct8::b), 2); + assert_eq!(align_of!(PackedStruct8::c), 4); + assert_eq!(align_of!(PackedStruct8::d), 8); + } +} From 971cba39d7fe70477512359bd1af76351a5e73f3 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 5 May 2024 22:23:43 -0400 Subject: [PATCH 09/13] Try some CI fixes after Cargo.toml changes --- .github/workflows/ci.yml | 20 ++++++++++---------- ctru-rs/Cargo.toml | 2 +- ctru-rs/build.rs | 1 + ctru-sys/Cargo.toml | 1 - ctru-sys/build.rs | 1 - ctru-sys/build/test_gen.rs | 1 - 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe58c0d1..1c28bccf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,15 +43,11 @@ jobs: - name: Check formatting run: cargo fmt --all --verbose -- --check - # We always run the next two steps, so that even if formatting fails we + # We always run the next step, so that even if formatting fails we # still get compilation errors for the same run (mainly for matchers). - - name: Cargo check ctru-sys (without tests) - run: cargo 3ds clippy --package ctru-sys --color=always --verbose - if: success() || failure() - - - name: Cargo check ctru-rs (including tests) - run: cargo 3ds clippy --package ctru-rs --color=always --verbose --all-targets + - name: Cargo check workspace (including tests / examples) + run: cargo 3ds clippy --workspace --color=always --verbose --all-targets if: success() || failure() test: @@ -80,17 +76,21 @@ jobs: # unless cargo-3ds actually runs them as separate commands. See # https://github.com/rust3ds/cargo-3ds/issues/44 for more details - name: Build lib and integration tests - run: cargo 3ds test --no-run --tests --all-features + # Hmm, I guess we really do need to build each target separately due to the above issue: + run: | + cargo 3ds test --no-run --lib --all-features --package ctru-sys + cargo 3ds test --no-run --test layout_test --all-features --package ctru-sys + cargo 3ds test --no-run --lib --all-features --package ctru-rs - name: Run lib and integration tests uses: rust3ds/test-runner/run-tests@v1 with: - args: --tests --all-features + args: --tests --all-features --workspace - name: Build and run doc tests uses: rust3ds/test-runner/run-tests@v1 with: - args: --doc + args: --doc --workspace - name: Upload citra logs and capture videos uses: actions/upload-artifact@v3 diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index 4e5883f9..c1e22930 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -22,7 +22,7 @@ ctru-sys = { path = "../ctru-sys", version = "0.5.0" } const-zero = "0.1.0" shim-3ds = { workspace = true } pthread-3ds = { workspace = true } -libc = { workspace = true } +libc = { workspace = true, default-features = true } bitflags = "2.3.3" macaddr = "1.0.1" widestring = "1.0.2" diff --git a/ctru-rs/build.rs b/ctru-rs/build.rs index f65d1948..d31a60eb 100644 --- a/ctru-rs/build.rs +++ b/ctru-rs/build.rs @@ -24,6 +24,7 @@ fn main() { let romfs_path = PathBuf::from(format!("{manifest_dir}/{romfs_dir_setting}")); // Check if the romfs path exists so we can compile the module + println!("cargo:rustc-check-cfg=cfg(romfs_exists)"); if romfs_path.exists() { println!("cargo:rustc-cfg=romfs_exists"); } diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index af93052f..c20f270b 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -26,7 +26,6 @@ layout-tests = [ ] [[test]] -path = "tests/layout_test.rs" name = "layout_test" required-features = ["layout-tests"] diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index db6f86f7..035ae113 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -170,7 +170,6 @@ fn main() { .blocklist_type("MiiData.*") .blocklist_type("ExHeader_(System|Arm11).*") .blocklist_type("FS_((Ext|System)SaveData|Program)Info") - .blocklist_type("FS_ProgramInfo") .blocklist_type("Y2RU_ConversionParams") .blocklist_field("romfs_(dir|file)", "name") // Bindgen generated types have no c++ equivalent: diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index c2112d45..d8179c92 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -17,7 +17,6 @@ use bindgen::callbacks::{ use bindgen::FieldVisibilityKind; use proc_macro2::TokenStream; use quote::{format_ident, quote, TokenStreamExt}; - use regex::Regex; use rust_format::{Formatter, RustFmt}; From 6019cd3a78cc0dd73efb8b15166a154f5fcef030 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Mon, 13 May 2024 21:59:30 -0400 Subject: [PATCH 10/13] Minor clippy fixups --- ctru-sys/build/test_gen.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index d8179c92..db0c97c5 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -109,7 +109,7 @@ impl LayoutTestGenerator { // the `#include` directives (they would be missing whitespace/newlines), // so we basically need to drop in the include headers here "manually" by // writing them into the cpp! macro invocation. - file.write_all("cpp! {{".as_bytes())?; + file.write_all("cpp! {{\n".as_bytes())?; for included_file in self.headers.borrow().iter() { writeln!(file, " #include \"{included_file}\"")?; } @@ -147,12 +147,12 @@ impl LayoutTestGenerator { let mut field_tests = Vec::new(); field_tests.push(build_assert_eq( - quote!(size_of!(#name)), - quote!(sizeof(#name)), + "e!(size_of!(#name)), + "e!(sizeof(#name)), )); field_tests.push(build_assert_eq( - quote!(align_of!(#name)), - quote!(alignof(#name)), + "e!(align_of!(#name)), + "e!(alignof(#name)), )); let struct_fields = self.struct_fields.borrow(); @@ -176,18 +176,18 @@ impl LayoutTestGenerator { let field = format_ident!("{field}"); field_tests.push(build_assert_eq( - quote!(size_of!(#name::#field)), - quote!(sizeof(#name::#field)), + "e!(size_of!(#name::#field)), + "e!(sizeof(#name::#field)), )); field_tests.push(build_assert_eq( - quote!(align_of!(#name::#field)), - quote!(alignof(#name::#field)), + "e!(align_of!(#name::#field)), + "e!(alignof(#name::#field)), )); field_tests.push(build_assert_eq( - quote!(offset_of!(#name, #field)), - quote!(offsetof(#name, #field)), + "e!(offset_of!(#name, #field)), + "e!(offsetof(#name, #field)), )); } } @@ -201,7 +201,7 @@ impl LayoutTestGenerator { } } -fn build_assert_eq(rust_lhs: TokenStream, cpp_rhs: TokenStream) -> TokenStream { +fn build_assert_eq(rust_lhs: &TokenStream, cpp_rhs: &TokenStream) -> TokenStream { quote! { assert_eq!( #rust_lhs, From 5c81deab2d4c42b31f50e9c09b32d21163375669 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Mon, 13 May 2024 22:11:28 -0400 Subject: [PATCH 11/13] Collect generated files in GHA artifacts --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c28bccf..32aa7156 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,3 +100,4 @@ jobs: path: | target/armv6k-nintendo-3ds/debug/deps/*.txt target/armv6k-nintendo-3ds/debug/deps/*.webm + target/armv6k-nintendo-3ds/debug/build/ctru-*/out/* From 752ae6adde5bde9743ab314ff0a4c3e690a5e2d2 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 17 May 2024 11:55:10 -0400 Subject: [PATCH 12/13] Address review comments and minor renaming --- Cargo.toml | 2 +- ctru-sys/Cargo.toml | 3 ++- ctru-sys/build/test_gen.rs | 3 +-- ctru-sys/tests/layout_test.rs | 9 +++++++-- test-runner/Cargo.toml | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 29d38753..caaf7866 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ default-members = ["ctru-rs", "ctru-sys"] resolver = "2" [workspace.dependencies] -libc = { version = "0.2.121", default-features = false } +libc = { version = "0.2.153", default-features = false } shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" } pthread-3ds = { git = "https://github.com/rust3ds/pthread-3ds.git" } test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" } diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index c20f270b..9253367e 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -35,6 +35,7 @@ libc = { workspace = true } [build-dependencies] bindgen = { version = "0.69", features = ["experimental"] } cc = "1.0" +# Use git dependency so we can use https://github.com/mystor/rust-cpp/pull/111 cpp_build = { optional = true, git = "https://github.com/mystor/rust-cpp.git" } doxygen-rs = "0.4.2" itertools = "0.11.0" @@ -47,7 +48,7 @@ which = "4.4.0" [dev-dependencies] shim-3ds = { workspace = true } test-runner = { workspace = true } -cpp = { git = "https://github.com/mystor/rust-cpp.git" } +cpp = "0.5.9" [package.metadata.docs.rs] default-target = "armv6k-nintendo-3ds" diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index db0c97c5..2609e778 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -143,7 +143,6 @@ impl LayoutTestGenerator { fn build_struct_test(&self, struct_name: &str) -> proc_macro2::TokenStream { let name = format_ident!("{struct_name}"); - let test_name = format_ident!("layout_test_{name}"); let mut field_tests = Vec::new(); field_tests.push(build_assert_eq( @@ -194,7 +193,7 @@ impl LayoutTestGenerator { quote! { #[test] - fn #test_name() { + fn #name() { #(#field_tests);* } } diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs index 88350fc0..55f8995a 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout_test.rs @@ -6,7 +6,6 @@ //! ABI used by libctru and the devkitARM toolchain, instead of just what libclang //! thinks they should be at bindgen time. -#![allow(non_snake_case)] #![feature(custom_test_frameworks)] #![test_runner(test_runner::run_gdb)] @@ -52,7 +51,13 @@ macro_rules! align_of { }; } -include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); +#[allow(non_snake_case)] +#[allow(non_upper_case_globals)] +mod generated { + use super::*; + + include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); +} mod helper_test { macro_rules! packed_struct { diff --git a/test-runner/Cargo.toml b/test-runner/Cargo.toml index 6d496994..c5d02d24 100644 --- a/test-runner/Cargo.toml +++ b/test-runner/Cargo.toml @@ -11,4 +11,4 @@ socket = [] [dependencies] ctru-rs = { git = "https://github.com/rust3ds/ctru-rs" } ctru-sys = { git = "https://github.com/rust3ds/ctru-rs" } -libc = "0.2.147" +libc = { workspace = true } From e4b02be9ae656c1e59d969c8da9efc3f9c1e254e Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 17 May 2024 13:17:50 -0400 Subject: [PATCH 13/13] Anchor regex and make test more comprehensive Blocklist specific bitfields etc instead of whole types, and document why each one is blocklisted. Also support field renames for mangling (e.g. `type`). --- ctru-sys/build.rs | 62 +++++++++++++++++++++++++---------- ctru-sys/build/test_gen.rs | 36 +++++++++++++------- ctru-sys/tests/layout_test.rs | 10 +++--- 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 035ae113..5b1fe3c0 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -162,27 +162,13 @@ fn main() { #[cfg(feature = "layout-tests")] { - let test_file = out_dir.join("generated_layout_test.rs"); - test_generator - // There are several bindgen-generated types that we don't want/need to check - // (because they are opaque, use bitfields, anonymous structs etc.) - .blocklist_type("Thread_tag") - .blocklist_type("MiiData.*") - .blocklist_type("ExHeader_(System|Arm11).*") - .blocklist_type("FS_((Ext|System)SaveData|Program)Info") - .blocklist_type("Y2RU_ConversionParams") - .blocklist_field("romfs_(dir|file)", "name") - // Bindgen generated types have no c++ equivalent: - .blocklist_type(".*__bindgen.*") - .blocklist_field(".*", "__bindgen.*") - // TODO: maybe we could translate type <-> `type_` or something... - .blocklist_field(".*", "type_") - .generate_layout_tests(&test_file) + let gen_test_file = out_dir.join("generated_layout_test.rs"); + generate_layout_tests(&gen_test_file, &test_generator) .unwrap_or_else(|err| panic!("Failed to generate layout tests: {err}")); cpp_build::Config::from(cc_build) - .compiler(&cpp) - .build(test_file); + .compiler(cpp) + .build(gen_test_file); } } @@ -285,3 +271,43 @@ fn track_libctru_files(pacman: &Path) -> Result<(), String> { Ok(()) } + +#[cfg(feature = "layout-tests")] +fn generate_layout_tests( + output_file: &Path, + test_generator: &build::test_gen::LayoutTestGenerator, +) -> Result<(), Box> { + // There are several bindgen-generated types/fields that we can't check: + test_generator + // Opaque types: + .blocklist_type("MiiData") + // Bitfields: + .blocklist_field( + "ExHeader_SystemInfoFlags", + "compress_exefs_code|is_sd_application", + ) + .blocklist_field( + "ExHeader_Arm11StorageInfo", + "reserved|no_romfs|use_extended_savedata_access", + ) + .blocklist_field( + "ExHeader_Arm11CoreInfo", + "use_cpu_clockrate_804MHz|enable_l2c|flag[12]_unused|[no]3ds_system_mode|ideal_processor|affinity_mask", + ) + .blocklist_field( + "Y2RU_ConversionParams", + "(input|output)_format|rotation|block_alignment|standard_coefficient", + ) + .blocklist_field( + "FS_(Program|(System|Ext)SaveData)Info", + "mediaType" + ) + // Variable-length arrays: + .blocklist_field("romfs_(dir|file)", "name") + // Bindgen anonymous types (and their associated fields): + .blocklist_type(".*__bindgen.*") + .blocklist_field(".*", "__bindgen.*") + // Bindgen mangles `type` (a Rust keyword) to `type_`: + .rename_field("type", "type_") + .generate_layout_tests(output_file) +} diff --git a/ctru-sys/build/test_gen.rs b/ctru-sys/build/test_gen.rs index 2609e778..eb907cb0 100644 --- a/ctru-sys/build/test_gen.rs +++ b/ctru-sys/build/test_gen.rs @@ -71,34 +71,44 @@ impl ParseCallbacks for LayoutTestCallbacks { #[derive(Debug)] pub struct LayoutTestGenerator { - struct_fields: RefCell>>, blocklist: RefCell)>>, headers: RefCell>, + renames: RefCell>, + struct_fields: RefCell>>, } impl LayoutTestGenerator { fn new() -> Self { Self { - struct_fields: RefCell::default(), blocklist: RefCell::default(), headers: RefCell::default(), + renames: RefCell::default(), + struct_fields: RefCell::default(), } } pub fn blocklist_type(&self, pattern: &str) -> &Self { self.blocklist .borrow_mut() - .push((Regex::new(pattern).unwrap(), None)); + .push((Regex::new(&format!("^({pattern})$")).unwrap(), None)); self } pub fn blocklist_field(&self, struct_pattern: &str, field_pattern: &str) -> &Self { self.blocklist.borrow_mut().push(( - Regex::new(struct_pattern).unwrap(), - Some(Regex::new(field_pattern).unwrap()), + Regex::new(&format!("^({struct_pattern})$")).unwrap(), + Some(Regex::new(&format!("^({field_pattern})$")).unwrap()), )); self } + + pub fn rename_field(&self, cpp_name: &str, rust_name: &str) -> &Self { + self.renames + .borrow_mut() + .insert(rust_name.to_string(), cpp_name.to_string()); + self + } + pub fn generate_layout_tests( &self, output_path: impl AsRef, @@ -172,21 +182,23 @@ impl LayoutTestGenerator { continue; } - let field = format_ident!("{field}"); + let rust_field = format_ident!("{field}"); + let cpp_field = + format_ident!("{}", self.renames.borrow().get(field).unwrap_or(field)); field_tests.push(build_assert_eq( - "e!(size_of!(#name::#field)), - "e!(sizeof(#name::#field)), + "e!(size_of!(#name::#rust_field)), + "e!(sizeof(#name::#cpp_field)), )); field_tests.push(build_assert_eq( - "e!(align_of!(#name::#field)), - "e!(alignof(#name::#field)), + "e!(align_of!(#name::#rust_field)), + "e!(alignof(#name::#cpp_field)), )); field_tests.push(build_assert_eq( - "e!(offset_of!(#name, #field)), - "e!(offsetof(#name, #field)), + "e!(offset_of!(#name, #rust_field)), + "e!(offsetof(#name, #cpp_field)), )); } } diff --git a/ctru-sys/tests/layout_test.rs b/ctru-sys/tests/layout_test.rs index 55f8995a..a92cec4b 100644 --- a/ctru-sys/tests/layout_test.rs +++ b/ctru-sys/tests/layout_test.rs @@ -13,9 +13,6 @@ extern crate shim_3ds; use std::mem::offset_of; -use cpp::cpp; -use ctru_sys::*; - fn size_of_ret(_f: impl Fn(U) -> T) -> usize { ::std::mem::size_of::() } @@ -53,13 +50,16 @@ macro_rules! align_of { #[allow(non_snake_case)] #[allow(non_upper_case_globals)] -mod generated { +mod generated_tests { use super::*; + use cpp::cpp; + use ctru_sys::*; + include!(concat!(env!("OUT_DIR"), "/generated_layout_test.rs")); } -mod helper_test { +mod helper_tests { macro_rules! packed_struct { ($name:ident, $size:literal) => { #[repr(C, packed($size))]