rust-bindgen icon indicating copy to clipboard operation
rust-bindgen copied to clipboard

Idea: Run expectation tests with specific clang versions

Open pvdrz opened this issue 11 months ago • 3 comments

Right now bindgen-tests will run with the system's clang version by default. Even though this can be overridden using environment variables, installing different clang versions can be cumbersome but not running the expectation tests could lead to unexpected CI failures. The most common example of this is a test that fails because clang 5 lacks a feature available in later clang versions.

I've had success using nix to install different clang versions without issue. However using a more ubiquitous tool like docker to set up the right environment would be more accessible and would reduce the time spent going back and forth with CI.

pvdrz avatar Sep 12 '23 16:09 pvdrz

Clang 16 also causes several tests to fail; at least:

  • header_constified_enum_module_overflow_hpp
  • header_issue_544_stylo_creduce_2_hpp
  • header_ptr32_has_different_size_h
  • header_struct_typedef_ns_hpp

The change for header_ptr32_has_different_size_h in particular seems like it represents a bugfix; the expected values seem wrong to me...

---- header_ptr32_has_different_size_h stdout ----
diff expected generated
--- expected: "/home/nathan/ext/rust-bindgen/bindgen-tests/tests/expectations/tests/libclang-9/ptr32-has-different-size
.rs"
+++ generated from: "/home/nathan/ext/rust-bindgen/bindgen-tests/tests/headers/ptr32-has-different-size.h"
2   2   |  #[repr(C)]
3   3   |  #[derive(Debug, Copy, Clone)]
4   4   |  pub struct TEST_STRUCT {
5       | -    pub ptr_32bit: *mut ::std::os::raw::c_void,
    5   | +    pub ptr_32bit: u32,
6   6   |  }
7   7   |  #[test]
8   8   |  fn bindgen_test_layout_TEST_STRUCT() {
(chunk 1/n)
10  10  |      let ptr = UNINIT.as_ptr();
11  11  |      assert_eq!(
12  12  |          ::std::mem::size_of::<TEST_STRUCT>(),
13      | -        8usize,
    13  | +        4usize,
14  14  |          concat!("Size of: ", stringify!(TEST_STRUCT)),
15  15  |      );
16  16  |      assert_eq!(
17  17  |          ::std::mem::align_of::<TEST_STRUCT>(),
18      | -        8usize,
    18  | +        4usize,
19  19  |          concat!("Alignment of ", stringify!(TEST_STRUCT)),
20  20  |      );
21  21  |      assert_eq!(
thread 'header_ptr32_has_different_size_h' panicked at /home/nathan/ext/rust-bindgen/target/debug/build/bindgen-tests-4
65c36707cc68643/out/tests.rs:409:1:
Header and binding differ! Run with BINDGEN_OVERWRITE_EXPECTED=1 in the environment to automatically overwrite the expe
ctation or with BINDGEN_TESTS_DIFFTOOL=meld to do this manually.

remexre avatar Jan 11 '24 23:01 remexre

I have some work towards this: https://github.com/rust-lang/rust-bindgen/compare/main...boydjohnson:rust-bindgen:feature/test-different-clang-versions

I used the debs available at apt.llvm.org for ubuntu:22.04 (jammy jellyfish).

Here are the bindgen-tests running in ci against libclang 13-18. https://github.com/boydjohnson/rust-bindgen/actions/runs/8290678113

The Dockerfiles I included are generally useful for local development, too.

These next to commands are to build the required images:

docker build -f dockerfiles/Dockerfile-clang --build-arg="LLVM_VERSION=18" -t clang:18-ubuntu .

docker build -f dockerfiles/Dockerfile-bindgen --build-arg="LLVM_VERSION=18" -t bindgen-tests:clang-18-ubuntu .

Then you can run local changes to bindgen through bindgen-tests via:

docker run -v $(pwd):/project bindgen-tests:clang-18-ubuntu

Is this well received and would you want me to build prior versions of llvm/libclang so that libclang 12 and prior can be tested against?

boydjohnson avatar Mar 15 '24 11:03 boydjohnson

This would certainly be appreciated. I can review a PR with these changes just ping me when you're ready.

pvdrz avatar Mar 26 '24 15:03 pvdrz