Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use absolute paths for unsaved files passed to clang and prepend -inc… #1857

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/lib.rs
Expand Up @@ -554,8 +554,16 @@ impl Builder {
///
/// The file `name` will be added to the clang arguments.
pub fn header_contents(mut self, name: &str, contents: &str) -> Builder {
// Apparently clang relies on having virtual FS correspondent to
// the real one, so we need absolute paths here
let absolute_path = env::current_dir()
.expect("Cannot retrieve current directory")
.join(name)
.to_str()
.expect("Cannot convert current directory name to string")
.to_owned();
self.input_header_contents
.push((name.into(), contents.into()));
.push((absolute_path, contents.into()));
self
}

Expand Down Expand Up @@ -1986,7 +1994,10 @@ impl Bindings {
}
}

for f in options.input_unsaved_files.iter() {
for (idx, f) in options.input_unsaved_files.iter().enumerate() {
if idx != 0 || options.input_header.is_some() {
options.clang_args.push("-include".to_owned());
}
options.clang_args.push(f.name.to_str().unwrap().to_owned())
}

Expand Down
111 changes: 111 additions & 0 deletions tests/expectations/tests/test_mixed_header_and_header_contents.rs
@@ -0,0 +1,111 @@
extern "C" {
pub static mut foo: ::std::option::Option<
unsafe extern "C" fn(
x: ::std::os::raw::c_int,
y: ::std::os::raw::c_int,
) -> ::std::os::raw::c_int,
>;
}
extern "C" {
pub fn bar(a: *const ::std::os::raw::c_char) -> ::std::os::raw::c_int;
}
extern "C" {
pub fn bar2(b: *const ::std::os::raw::c_char) -> f32;
}
pub type Char = ::std::os::raw::c_char;
pub type SChar = ::std::os::raw::c_schar;
pub type UChar = ::std::os::raw::c_uchar;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct Test {
pub ch: ::std::os::raw::c_char,
pub u: ::std::os::raw::c_uchar,
pub d: ::std::os::raw::c_schar,
pub cch: ::std::os::raw::c_char,
pub cu: ::std::os::raw::c_uchar,
pub cd: ::std::os::raw::c_schar,
pub Cch: Char,
pub Cu: UChar,
pub Cd: SChar,
pub Ccch: Char,
pub Ccu: UChar,
pub Ccd: SChar,
}
#[test]
fn bindgen_test_layout_Test() {
assert_eq!(
::std::mem::size_of::<Test>(),
12usize,
concat!("Size of: ", stringify!(Test))
);
assert_eq!(
::std::mem::align_of::<Test>(),
1usize,
concat!("Alignment of ", stringify!(Test))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).ch as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(ch))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).u as *const _ as usize },
1usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(u))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).d as *const _ as usize },
2usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(d))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).cch as *const _ as usize },
3usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(cch))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).cu as *const _ as usize },
4usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(cu))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).cd as *const _ as usize },
5usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(cd))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Cch as *const _ as usize },
6usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(Cch))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Cu as *const _ as usize },
7usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(Cu))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Cd as *const _ as usize },
8usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(Cd))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Ccch as *const _ as usize },
9usize,
concat!(
"Offset of field: ",
stringify!(Test),
"::",
stringify!(Ccch)
)
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Ccu as *const _ as usize },
10usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(Ccu))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Test>())).Ccd as *const _ as usize },
11usize,
concat!("Offset of field: ", stringify!(Test), "::", stringify!(Ccd))
);
}
55 changes: 55 additions & 0 deletions tests/tests.rs
Expand Up @@ -454,6 +454,61 @@ fn test_multiple_header_calls_in_builder() {
}
}

#[test]
fn test_multiple_header_contents() {
let actual = builder()
.header_contents("test.h", "int foo(const char* a);")
.header_contents("test2.h", "float foo2(const char* b);")
.clang_arg("--target=x86_64-unknown-linux")
.generate()
.unwrap()
.to_string();

let (actual, stderr) = rustfmt(actual);
println!("{}", stderr);

let (expected, _) = rustfmt(
"extern \"C\" {
pub fn foo2(b: *const ::std::os::raw::c_char) -> f32;
}
extern \"C\" {
pub fn foo(a: *const ::std::os::raw::c_char) -> ::std::os::raw::c_int;
}
"
.to_string(),
);

assert_eq!(expected, actual);
}

#[test]
fn test_mixed_header_and_header_contents() {
let actual = builder()
.header(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/headers/func_ptr.h"
))
.header(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/headers/char.h"))
.header_contents("test.h", "int bar(const char* a);")
.header_contents("test2.h", "float bar2(const char* b);")
.clang_arg("--target=x86_64-unknown-linux")
.generate()
.unwrap()
.to_string();

let (actual, stderr) = rustfmt(actual);
println!("{}", stderr);

let expected = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/test_mixed_header_and_header_contents.rs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how do you update this file when output changes? Do you need to do that manually? If so, it's a bit unfortunate.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for review :) And could you elaborate on this issue, please? I'm not sure what are the other ways to update the expectation but to do it manually, unfortunately. E.g., in test_multiple_header_calls_in_builder the pattern is the same, if I got it right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right that this pattern is not new, it's just worse having tests you need to manually update, because the overhead of updating them scales with the amount of tests :(

I guess we can take this as-is though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now that I think about it - I can use two existing test expectation files and try to concatenate them with inline expectation from header_contents calls, so that I will not introduce new expectation files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds great. as long as the first header doesn't have any anonymous enum or struct it should work.

));
let (expected, _) = rustfmt(expected.to_string());

assert_eq!(expected, actual);
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to remove this stray newline for CI to pass.

#[test]
// Doesn't support executing sh file on Windows.
// We may want to implement it in Rust so that we support all systems.
Expand Down