Merge pull request #4554 from tgross35/ctest-tweaks

ctest: Small adjustments to existing tests

Author: tgross35

.github/workflows/ci.yaml

@@ -85,8 +85,13 @@ jobs:
       - name: Execute build.sh
         run: |
           set -eux
-          # Remove `-Dwarnings` at the MSRV since lints may be different
-          [ "${{ matrix.toolchain }}" = "1.63.0" ] && export RUSTFLAGS=""
+          if [ "${{ matrix.toolchain }}" = "1.63.0" ]; then
+              # Remove `-Dwarnings` at the MSRV since lints may be different
+              export RUSTFLAGS=""
+              # Remove `ctest-next` which uses the 2024 edition
+              perl -i -ne 'print unless /"ctest-next",/' Cargo.toml
+          fi
+
           ./ci/verify-build.sh
       - name: Target size after job completion
         run: du -sh target | sort -k 2
@@ -314,6 +319,8 @@ jobs:
         echo "MSRV=$msrv" >> "$GITHUB_ENV"
     - name: Install Rust
       run: rustup update "$MSRV" --no-self-update && rustup default "$MSRV"
+    - name: Remove edition 2024 crates
+      run: perl -i -ne 'print unless /"ctest-next",/' Cargo.toml
     - uses: Swatinem/rust-cache@v2
     - run: cargo build -p ctest
 

ctest-next/Cargo.toml

@@ -1,8 +1,8 @@
 [package]
 name = "ctest-next"
 version = "0.1.0"
-edition = "2021"
-rust-version = "1.87"
+edition = "2024"
+rust-version = "1.88"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-lang/libc"
 publish = false

ctest-next/src/generator.rs

@@ -10,7 +10,8 @@ use thiserror::Error;
 use crate::ffi_items::FfiItems;
 use crate::template::{CTestTemplate, RustTestTemplate};
 use crate::{
-    expand, Const, Field, MapInput, Parameter, Result, Static, Struct, Type, VolatileItemKind,
+    Const, Field, MapInput, Parameter, Result, Static, Struct, TranslationError, Type,
+    VolatileItemKind, expand,
 };
 
 /// A function that takes a mappable input and returns its mapping as Some, otherwise
@@ -46,6 +47,12 @@ pub enum GenerationError {
     MacroExpansion(PathBuf, String),
     #[error("unable to parse expanded crate {0}: {1}")]
     RustSyntax(String, String),
+    #[error("unable to prepare template input: {0}")]
+    Translation(#[from] TranslationError),
+    #[error("unable to render Rust template: {0}")]
+    RustTemplateRender(askama::Error),
+    #[error("unable to render C template: {0}")]
+    CTemplateRender(askama::Error),
     #[error("unable to render {0} template: {1}")]
     TemplateRender(String, String),
     #[error("unable to create or write template file: {0}")]
@@ -600,33 +607,32 @@ impl TestGenerator {
             .unwrap_or_else(|| env::var("OUT_DIR").unwrap().into());
         let output_file_path = output_directory.join(output_file_path);
 
+        let ensure_trailing_newline = |s: &mut String| {
+            s.truncate(s.trim_end().len());
+            s.push('\n');
+        };
+
+        let mut rust_file = RustTestTemplate::new(&ffi_items, self)?
+            .render()
+            .map_err(GenerationError::RustTemplateRender)?;
+        ensure_trailing_newline(&mut rust_file);
+
         // Generate the Rust side of the tests.
         File::create(output_file_path.with_extension("rs"))
             .map_err(GenerationError::OsError)?
-            .write_all(
-                RustTestTemplate::new(&ffi_items, self)
-                    .map_err(|e| {
-                        GenerationError::TemplateRender("Rust".to_string(), e.to_string())
-                    })?
-                    .render()
-                    .map_err(|e| {
-                        GenerationError::TemplateRender("Rust".to_string(), e.to_string())
-                    })?
-                    .as_bytes(),
-            )
+            .write_all(rust_file.as_bytes())
             .map_err(GenerationError::OsError)?;
 
+        let mut c_file = CTestTemplate::new(&ffi_items, self)?
+            .render()
+            .map_err(GenerationError::CTemplateRender)?;
+        ensure_trailing_newline(&mut c_file);
+
         // Generate the C/Cxx side of the tests.
         let c_output_path = output_file_path.with_extension("c");
         File::create(&c_output_path)
             .map_err(GenerationError::OsError)?
-            .write_all(
-                CTestTemplate::new(&ffi_items, self)
-                    .map_err(|e| GenerationError::TemplateRender("C".to_string(), e.to_string()))?
-                    .render()
-                    .map_err(|e| GenerationError::TemplateRender("C".to_string(), e.to_string()))?
-                    .as_bytes(),
-            )
+            .write_all(c_file.as_bytes())
             .map_err(GenerationError::OsError)?;
 
         Ok(output_file_path)

ctest-next/src/runner.rs

@@ -1,5 +1,5 @@
 use std::env;
-use std::fs::{canonicalize, File};
+use std::fs::{File, canonicalize};
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::Command;

ctest-next/src/template.rs

@@ -9,7 +9,7 @@ use crate::{BoxStr, MapInput, Result, TestGenerator, TranslationError};
 #[derive(Template, Clone)]
 #[template(path = "test.rs")]
 pub(crate) struct RustTestTemplate {
-    pub(crate) template: TestTemplate,
+    pub template: TestTemplate,
 }
 
 impl RustTestTemplate {
@@ -27,8 +27,8 @@ impl RustTestTemplate {
 #[derive(Template, Clone)]
 #[template(path = "test.c")]
 pub(crate) struct CTestTemplate {
-    pub(crate) template: TestTemplate,
-    pub(crate) headers: Vec<String>,
+    pub template: TestTemplate,
+    pub headers: Vec<String>,
 }
 
 impl CTestTemplate {
@@ -46,9 +46,9 @@ impl CTestTemplate {
 /// Stores all information necessary for generation of tests for all items.
 #[derive(Clone, Debug, Default)]
 pub(crate) struct TestTemplate {
-    pub(crate) const_cstr_tests: Vec<TestCstr>,
-    pub(crate) const_tests: Vec<TestConst>,
-    pub(crate) test_idents: Vec<BoxStr>,
+    pub const_cstr_tests: Vec<TestCStr>,
+    pub const_tests: Vec<TestConst>,
+    pub test_idents: Vec<BoxStr>,
 }
 
 impl TestTemplate {
@@ -71,37 +71,34 @@ impl TestTemplate {
         let mut const_tests = vec![];
         let mut const_cstr_tests = vec![];
         for constant in ffi_items.constants() {
-            if let syn::Type::Ptr(ptr) = &constant.ty {
-                let is_const_c_char_ptr = matches!(
-                    &*ptr.elem,
-                    syn::Type::Path(path)
-                        if path.path.segments.last().unwrap().ident == "c_char"
-                        && ptr.mutability.is_none()
-                );
-                if is_const_c_char_ptr {
-                    let item = TestCstr {
-                        test_ident: cstr_test_ident(constant.ident()),
-                        rust_ident: constant.ident().into(),
-                        c_ident: helper.c_ident(constant).into(),
-                        c_type: helper.c_type(constant)?.into(),
-                    };
-                    const_cstr_tests.push(item)
-                }
+            if let syn::Type::Ptr(ptr) = &constant.ty
+                && let syn::Type::Path(path) = &*ptr.elem
+                && path.path.segments.last().unwrap().ident == "c_char"
+                && ptr.mutability.is_none()
+            {
+                let item = TestCStr {
+                    id: constant.ident().into(),
+                    test_name: cstr_test_ident(constant.ident()),
+                    rust_val: constant.ident().into(),
+                    c_val: helper.c_ident(constant).into(),
+                };
+                const_cstr_tests.push(item)
             } else {
                 let item = TestConst {
-                    test_ident: const_test_ident(constant.ident()),
-                    rust_ident: constant.ident.clone(),
-                    rust_type: constant.ty.to_token_stream().to_string().into_boxed_str(),
-                    c_ident: helper.c_ident(constant).into(),
-                    c_type: helper.c_type(constant)?.into(),
+                    id: constant.ident().into(),
+                    test_name: const_test_ident(constant.ident()),
+                    rust_val: constant.ident.clone(),
+                    rust_ty: constant.ty.to_token_stream().to_string().into_boxed_str(),
+                    c_val: helper.c_ident(constant).into(),
+                    c_ty: helper.c_type(constant)?.into(),
                 };
                 const_tests.push(item)
             }
         }
 
         let mut test_idents = vec![];
-        test_idents.extend(const_cstr_tests.iter().map(|test| test.test_ident.clone()));
-        test_idents.extend(const_tests.iter().map(|test| test.test_ident.clone()));
+        test_idents.extend(const_cstr_tests.iter().map(|test| test.test_name.clone()));
+        test_idents.extend(const_tests.iter().map(|test| test.test_name.clone()));
 
         Ok(Self {
             const_cstr_tests,
@@ -111,23 +108,35 @@ impl TestTemplate {
     }
 }
 
+/* Many test structures have the following fields:
+ *
+ * - `test_name`: The function name.
+ * - `id`: An identifier that can be used to create functions related to this type without conflict,
+ *    usually also part of `test_name`.
+ * - `rust_val`: Identifier for a Rust value, with path qualifications if needed.
+ * - `rust_ty`: The Rust type of the relevant item, with path qualifications if needed.
+ * - `c_val`: Identifier for a C value (e.g. `#define`)
+ * - `c_ty`: The C type of the constant, qualified with `struct` or `union` if needed.
+ */
+
 /// Information required to test a constant CStr.
 #[derive(Clone, Debug)]
-pub(crate) struct TestCstr {
-    pub(crate) test_ident: BoxStr,
-    pub(crate) rust_ident: BoxStr,
-    pub(crate) c_ident: BoxStr,
-    pub(crate) c_type: BoxStr,
+pub(crate) struct TestCStr {
+    pub test_name: BoxStr,
+    pub id: BoxStr,
+    pub rust_val: BoxStr,
+    pub c_val: BoxStr,
 }
 
 /// Information required to test a constant.
 #[derive(Clone, Debug)]
 pub(crate) struct TestConst {
-    pub(crate) test_ident: BoxStr,
-    pub(crate) rust_ident: BoxStr,
-    pub(crate) rust_type: BoxStr,
-    pub(crate) c_ident: BoxStr,
-    pub(crate) c_type: BoxStr,
+    pub test_name: BoxStr,
+    pub id: BoxStr,
+    pub rust_val: BoxStr,
+    pub c_val: BoxStr,
+    pub rust_ty: BoxStr,
+    pub c_ty: BoxStr,
 }
 
 /// The Rust name of the cstr test.

ctest-next/src/translator.rs

@@ -67,7 +67,9 @@ pub(crate) enum TranslationErrorKind {
     HasLifetimes,
 
     /// A type that is not ffi compatible was found.
-    #[error("this type is not guaranteed to have a C compatible layout. See improper_ctypes_definitions lint")]
+    #[error(
+        "this type is not guaranteed to have a C compatible layout. See improper_ctypes_definitions lint"
+    )]
     NotFfiCompatible,
 }
 

ctest-next/templates/test.c

@@ -15,23 +15,22 @@
 
 {%- for const_cstr in ctx.const_cstr_tests +%}
 
-static {{ const_cstr.c_type }} ctest_const_{{ const_cstr.rust_ident }}_val_static = {{ const_cstr.c_ident }};
+static char *ctest_const_{{ const_cstr.id }}_val_static = {{ const_cstr.c_val }};
 
 // Define a function that returns a pointer to the value of the constant to test.
 // This will later be called on the Rust side via FFI.
-{{ const_cstr.c_type }}* __{{ const_cstr.test_ident }}(void) {
-    return &ctest_const_{{ const_cstr.rust_ident }}_val_static;
+char *ctest_const_cstr__{{ const_cstr.id }}(void) {
+    return ctest_const_{{ const_cstr.id }}_val_static;
 }
 {%- endfor +%}
 
 {%- for constant in ctx.const_tests +%}
 
-static {{ constant.c_type }} ctest_const_{{ constant.rust_ident }}_val_static = {{ constant.c_ident }};
+static {{ constant.c_ty }} ctest_const_{{ constant.id }}_val_static = {{ constant.c_val }};
 
 // Define a function that returns a pointer to the value of the constant to test.
 // This will later be called on the Rust side via FFI.
-{{ constant.c_type }}* __{{ constant.test_ident }}(void) {
-    return &ctest_const_{{ constant.rust_ident }}_val_static;
+{{ constant.c_ty }} *ctest_const__{{ constant.id }}(void) {
+    return &ctest_const_{{ constant.id }}_val_static;
 }
 {%- endfor +%}
-

ctest-next/templates/test.rs

@@ -44,48 +44,60 @@ mod generated_tests {
         }
     }
 
-    {%- for const_cstr in ctx.const_cstr_tests +%}
+{%- for const_cstr in ctx.const_cstr_tests +%}
 
     // Test that the string constant is the same in both Rust and C.
     // While fat pointers can't be translated, we instead use * const c_char.
-    pub fn {{ const_cstr.test_ident }}() {
+    pub fn {{ const_cstr.test_name }}() {
         extern "C" {
-            fn __{{ const_cstr.test_ident }}() -> *const *const u8;
-        }
-        let val = {{ const_cstr.rust_ident }};
-        unsafe {
-            let ptr = *__{{ const_cstr.test_ident }}();
-            let val = CStr::from_ptr(ptr.cast::<c_char>());
-            let val = val.to_str().expect("const {{ const_cstr.rust_ident }} not utf8");
-            let c = ::std::ffi::CStr::from_ptr(ptr as *const _);
-            let c = c.to_str().expect("const {{ const_cstr.rust_ident }} not utf8");
-            check_same(val, c, "{{ const_cstr.rust_ident }} string");
+            fn ctest_const_cstr__{{ const_cstr.id }}() -> *const c_char;
         }
+
+        // SAFETY: we assume that `c_char` pointer consts are for C strings.
+        let r_val = unsafe {
+            let r_ptr: *const c_char = {{ const_cstr.rust_val }};
+            assert!(!r_ptr.is_null(), "const {{ const_cstr.rust_val }} is null");
+            CStr::from_ptr(r_ptr)
+        };
+
+        // SAFETY: FFI call returns a valid C string.
+        let c_val = unsafe {
+            let c_ptr: *const c_char = unsafe { ctest_const_cstr__{{ const_cstr.id }}()  };
+            CStr::from_ptr(c_ptr)
+        };
+
+        check_same(r_val, c_val, "const {{ const_cstr.rust_val }} string");
     }
-    {%- endfor +%}
+{%- endfor +%}
 
-    {%- for constant in ctx.const_tests +%}
+{%- for constant in ctx.const_tests +%}
 
     // Test that the value of the constant is the same in both Rust and C.
     // This performs a byte by byte comparision of the constant value.
-    pub fn {{ constant.test_ident }}() {
+    pub fn {{ constant.test_name }}() {
+        type T = {{ constant.rust_ty }};
         extern "C" {
-            fn __{{ constant.test_ident }}() -> *const {{ constant.rust_type }};
+            fn ctest_const__{{ constant.id }}() -> *const T;
         }
-        let val = {{ constant.rust_ident }};
-        unsafe {
-            let ptr1 = ptr::from_ref(&val).cast::<u8>();
-            let ptr2 = __{{ constant.test_ident }}().cast::<u8>();
-            let ptr1_bytes = slice::from_raw_parts(ptr1, mem::size_of::<{{ constant.rust_type }}>());
-            let ptr2_bytes = slice::from_raw_parts(ptr2, mem::size_of::<{{ constant.rust_type }}>());
-            for (i, (&b1, &b2)) in ptr1_bytes.iter().zip(ptr2_bytes.iter()).enumerate() {
-                // HACK: This may read uninitialized data! We do this because
-                // there isn't a good way to recursively iterate all fields.
-                check_same_hex(b1, b2, &format!("{{ constant.rust_ident }} value at byte {}", i));
-            }
+
+        /* HACK: The slices may contian uninitialized data! We do this because
+         * there isn't a good way to recursively iterate all fields. */
+
+        let r_val: T = {{ constant.rust_val }};
+        let r_bytes = unsafe {
+            slice::from_raw_parts(ptr::from_ref(&r_val).cast::<u8>(), size_of::<T>())
+        };
+
+        let c_bytes = unsafe {
+            let c_ptr: *const T = unsafe { ctest_const__{{ constant.id }}() };
+            slice::from_raw_parts(c_ptr.cast::<u8>(), size_of::<T>())
+        };
+
+        for (i, (&b1, &b2)) in r_bytes.iter().zip(c_bytes.iter()).enumerate() {
+            check_same_hex(b1, b2, &format!("{{ constant.rust_val }} value at byte {}", i));
         }
     }
-    {%- endfor +%}
+{%- endfor +%}
 }
 
 use generated_tests::*;
@@ -109,4 +121,3 @@ fn run_all() {
     {{ test }}();
     {%- endfor +%}
 }
-