Merge pull request #181 from rust3ds/feature/cpp-sizeof-tests

.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
+        # 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
+          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
@@ -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/*

Cargo.toml

@@ -3,13 +3,19 @@ members = ["ctru-rs", "ctru-sys", "test-runner"]
 default-members = ["ctru-rs", "ctru-sys"]
 resolver = "2"
 
-[patch.'https://github.com/rust3ds/ctru-rs']
+[workspace.dependencies]
+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" }
+
+[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" }
-test-runner = { path = "test-runner" }
 
 # 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" }

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, default-features = true }
 bitflags = "2.3.3"
 macaddr = "1.0.1"
 widestring = "1.0.2"

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");
     }

ctru-sys/Cargo.toml

@@ -12,19 +12,43 @@ license = "Zlib"
 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 = [
+    "dep:cpp_build",
+    "dep:proc-macro2",
+    "dep:quote",
+    "dep:regex",
+    "dep:rust-format",
+]
+
+[[test]]
+name = "layout_test"
+required-features = ["layout-tests"]
+
 [dependencies]
-libc = { version = "0.2.121", default-features = false }
+libc = { workspace = true }
 
 [build-dependencies]
-bindgen = { version = "0.65.1", features = ["experimental"] }
+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"
+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"
 
 [dev-dependencies]
-shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" }
-test-runner = { git = "https://github.com/rust3ds/ctru-rs.git" }
+shim-3ds = { workspace = true }
+test-runner = { workspace = true }
+cpp = "0.5.9"
 
 [package.metadata.docs.rs]
 default-target = "armv6k-nintendo-3ds"

ctru-sys/build.rs

@@ -7,6 +7,13 @@ 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;
+}
+
 #[derive(Debug)]
 struct CustomCallbacks;
 
@@ -53,20 +60,63 @@ 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");
 
-    let include_path = PathBuf::from_iter([devkitpro.as_str(), "libctru", "include"]);
+    #[cfg(feature = "layout-tests")]
+    let cpp = bin_dir.join("arm-none-eabi-g++");
+
+    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");
 
+    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 cc_build = cc::Build::new();
+    cc_build
+        .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 = cc_build
+        .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();
+
     // 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)
@@ -92,59 +142,37 @@ 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(CustomCallbacks))
-        .generate()
-        .expect("unable to generate bindings");
+        .clang_args(clang.args().iter().map(|s| s.to_str().unwrap()))
+        .parse_callbacks(Box::new(CustomCallbacks));
 
-    bindings
+    #[cfg(feature = "layout-tests")]
+    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));
+
+    binding_builder
+        .generate()
+        .expect("unable to generate bindings")
         .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)
+    cc_build
         .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");
+
+    #[cfg(feature = "layout-tests")]
+    {
+        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(gen_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())
@@ -232,7 +260,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 +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<dyn Error>> {
+    // 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)
+}