From e847ef1a646016ca007fb4f78e079544dac75ae6 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Mon, 29 Apr 2024 10:34:43 -0400 Subject: [PATCH] Switch to single precompiled header in macro fallback It appears that Clang only supports a single precompiled header at a time. Because the macro fallback depends on the ability to provide multiple precompiled headers at once, this commit changes the code to include all provided headers into a single header to precompile and then pass to the TranslationUnit. This should resolve the issue where the macro fallback would not function as intended when multiple headers were provided as input. This commit also resolves an issue where clang args passed to the builder were not forwarded to the precompilation translation unit, resulting in headers not in standard system directories not being found. --- .../macro_fallback_non_system_dir.rs | 1 + .../test_macro_fallback_non_system_dir.rs | 6 + bindgen-tests/tests/headers/issue-753.h | 5 + .../another_header.h | 10 ++ .../macro_fallback_test_headers/one_header.h | 8 ++ bindgen-tests/tests/tests.rs | 55 ++++++++ bindgen/clang.rs | 14 +- bindgen/ir/context.rs | 124 ++++++++++++------ 8 files changed, 174 insertions(+), 49 deletions(-) create mode 100644 bindgen-tests/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs create mode 100644 bindgen-tests/tests/expectations/tests/test_macro_fallback_non_system_dir.rs create mode 100644 bindgen-tests/tests/macro_fallback_test_headers/another_header.h create mode 100644 bindgen-tests/tests/macro_fallback_test_headers/one_header.h diff --git a/bindgen-tests/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs b/bindgen-tests/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs new file mode 100644 index 0000000000..8f5c4ba2da --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs @@ -0,0 +1 @@ +pub const NEGATIVE: i32 = -1; diff --git a/bindgen-tests/tests/expectations/tests/test_macro_fallback_non_system_dir.rs b/bindgen-tests/tests/expectations/tests/test_macro_fallback_non_system_dir.rs new file mode 100644 index 0000000000..bf9739f3fa --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/test_macro_fallback_non_system_dir.rs @@ -0,0 +1,6 @@ +pub const CONST: u32 = 5; +pub const OTHER_CONST: u32 = 6; +pub const LARGE_CONST: u32 = 1536; +pub const THE_CONST: u32 = 28; +pub const MY_CONST: u32 = 69; +pub const NEGATIVE: i32 = -1; diff --git a/bindgen-tests/tests/headers/issue-753.h b/bindgen-tests/tests/headers/issue-753.h index 94bb8e1c31..3a6c82528a 100644 --- a/bindgen-tests/tests/headers/issue-753.h +++ b/bindgen-tests/tests/headers/issue-753.h @@ -1,7 +1,12 @@ // bindgen-flags: --clang-macro-fallback +#ifndef ISSUE_753_H +#define ISSUE_753_H + #define UINT32_C(c) c ## U #define CONST UINT32_C(5) #define OTHER_CONST UINT32_C(6) #define LARGE_CONST UINT32_C(6 << 8) + +#endif diff --git a/bindgen-tests/tests/macro_fallback_test_headers/another_header.h b/bindgen-tests/tests/macro_fallback_test_headers/another_header.h new file mode 100644 index 0000000000..b0c40eb43f --- /dev/null +++ b/bindgen-tests/tests/macro_fallback_test_headers/another_header.h @@ -0,0 +1,10 @@ +#ifndef ANOTHER_HEADER_H +#define ANOTHER_HEADER_H + +#include + +#define SHOULD_NOT_GENERATE UINT64_C(~0) +#define MY_CONST UINT32_C(69) +#define NEGATIVE ~0 + +#endif diff --git a/bindgen-tests/tests/macro_fallback_test_headers/one_header.h b/bindgen-tests/tests/macro_fallback_test_headers/one_header.h new file mode 100644 index 0000000000..5058814bba --- /dev/null +++ b/bindgen-tests/tests/macro_fallback_test_headers/one_header.h @@ -0,0 +1,8 @@ +#ifndef ONE_HEADER_H +#define ONE_HEADER_H + +#include + +#define THE_CONST UINT32_C(28) + +#endif diff --git a/bindgen-tests/tests/tests.rs b/bindgen-tests/tests/tests.rs index e6c038a064..14988e463f 100644 --- a/bindgen-tests/tests/tests.rs +++ b/bindgen-tests/tests/tests.rs @@ -565,6 +565,61 @@ fn test_mixed_header_and_header_contents() { } } +#[test] +fn test_macro_fallback_non_system_dir() { + let actual = builder() + .header(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/macro_fallback_test_headers/one_header.h" + )) + .header(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/macro_fallback_test_headers/another_header.h" + )) + .clang_macro_fallback() + .clang_arg(format!("-I{}/tests/headers", env!("CARGO_MANIFEST_DIR"))) + .generate() + .unwrap() + .to_string(); + + let actual = format_code(actual).unwrap(); + + let (expected_filename, expected) = match clang_version().parsed { + Some((9, _)) => { + let expected_filename = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs", + ); + let expected = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs", + )); + (expected_filename, expected) + } + _ => { + let expected_filename = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/expectations/tests/test_macro_fallback_non_system_dir.rs", + ); + let expected = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/expectations/tests/test_macro_fallback_non_system_dir.rs", + )); + (expected_filename, expected) + } + }; + let expected = format_code(expected).unwrap(); + if expected != actual { + error_diff_mismatch( + &actual, + &expected, + None, + Path::new(expected_filename), + ) + .unwrap(); + } +} + #[test] // Doesn't support executing sh file on Windows. // We may want to implement it in Rust so that we support all systems. diff --git a/bindgen/clang.rs b/bindgen/clang.rs index 08d3381557..26c02acec9 100644 --- a/bindgen/clang.rs +++ b/bindgen/clang.rs @@ -1908,7 +1908,8 @@ impl Drop for TranslationUnit { /// Translation unit used for macro fallback parsing pub(crate) struct FallbackTranslationUnit { file_path: String, - pch_paths: Vec, + header_path: String, + pch_path: String, idx: Box, tu: TranslationUnit, } @@ -1923,7 +1924,8 @@ impl FallbackTranslationUnit { /// Create a new fallback translation unit pub(crate) fn new( file: String, - pch_paths: Vec, + header_path: String, + pch_path: String, c_args: &[Box], ) -> Option { // Create empty file @@ -1944,7 +1946,8 @@ impl FallbackTranslationUnit { )?; Some(FallbackTranslationUnit { file_path: file, - pch_paths, + header_path, + pch_path, tu: f_translation_unit, idx: f_index, }) @@ -1982,9 +1985,8 @@ impl FallbackTranslationUnit { impl Drop for FallbackTranslationUnit { fn drop(&mut self) { let _ = std::fs::remove_file(&self.file_path); - for pch in self.pch_paths.iter() { - let _ = std::fs::remove_file(pch); - } + let _ = std::fs::remove_file(&self.header_path); + let _ = std::fs::remove_file(&self.pch_path); } } diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 2e608b0714..a1536935b6 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -29,6 +29,8 @@ use quote::ToTokens; use std::borrow::Cow; use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap as StdHashMap}; +use std::fs::OpenOptions; +use std::io::Write; use std::mem; use std::path::Path; @@ -2081,55 +2083,91 @@ If you encounter an error missing from this list, please file an issue or a PR!" let index = clang::Index::new(false, false); - let mut c_args = Vec::new(); - let mut pch_paths = Vec::new(); - for input_header in self.options().input_headers.iter() { + let mut header_names_to_compile = Vec::new(); + let mut header_paths = Vec::new(); + let mut header_contents = String::new(); + for input_header in self.options.input_headers.iter() { let path = Path::new(input_header.as_ref()); - let header_name = path - .file_name() - .and_then(|hn| hn.to_str()) - .map(|s| s.to_owned()); - let header_path = path - .parent() - .and_then(|hp| hp.to_str()) - .map(|s| s.to_owned()); - - let (header, pch) = if let (Some(ref hp), Some(hn)) = - (header_path, header_name) - { - let header_path = if hp.is_empty() { "." } else { hp }; - let header = format!("{header_path}/{hn}"); - let pch_path = if let Some(ref path) = - self.options().clang_macro_fallback_build_dir - { - path.as_os_str().to_str()? + if let Some(header_path) = path.parent() { + if header_path == Path::new("") { + header_paths.push("."); } else { - header_path - }; - (header, format!("{pch_path}/{hn}.pch")) + header_paths.push(header_path.as_os_str().to_str()?); + } } else { - return None; - }; - - let mut tu = clang::TranslationUnit::parse( - &index, - &header, - &[ - "-x".to_owned().into_boxed_str(), - "c-header".to_owned().into_boxed_str(), - ], - &[], - clang_sys::CXTranslationUnit_ForSerialization, - )?; - tu.save(&pch).ok()?; - - c_args.push("-include-pch".to_string().into_boxed_str()); - c_args.push(pch.clone().into_boxed_str()); - pch_paths.push(pch); + header_paths.push("."); + } + let header_name = path.file_name()?.to_str()?; + header_names_to_compile + .push(header_name.split(".h").next()?.to_string()); + header_contents += + format!("\n#include <{header_name}>").as_str(); } + let header_to_precompile = format!( + "{}/{}", + match self.options().clang_macro_fallback_build_dir { + Some(ref path) => path.as_os_str().to_str()?, + None => ".", + }, + header_names_to_compile.join("-") + "-precompile.h" + ); + let pch = header_to_precompile.clone() + ".pch"; + + let mut header_to_precompile_file = OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&header_to_precompile) + .ok()?; + header_to_precompile_file + .write_all(header_contents.as_bytes()) + .ok()?; + let mut c_args = Vec::new(); + c_args.push("-x".to_string().into_boxed_str()); + c_args.push("c-header".to_string().into_boxed_str()); + for header_path in header_paths { + c_args.push(format!("-I{header_path}").into_boxed_str()); + } + c_args.extend( + self.options + .clang_args + .iter() + .filter(|next| { + !self.options.input_headers.contains(next) && + next.as_ref() != "-include" + }) + .cloned(), + ); + let mut tu = clang::TranslationUnit::parse( + &index, + &header_to_precompile, + &c_args, + &[], + clang_sys::CXTranslationUnit_ForSerialization, + )?; + tu.save(&pch).ok()?; + + let mut c_args = vec![ + "-include-pch".to_string().into_boxed_str(), + pch.clone().into_boxed_str(), + ]; + c_args.extend( + self.options + .clang_args + .clone() + .iter() + .filter(|next| { + !self.options.input_headers.contains(next) && + next.as_ref() != "-include" + }) + .cloned(), + ); self.fallback_tu = Some(clang::FallbackTranslationUnit::new( - file, pch_paths, &c_args, + file, + header_to_precompile, + pch, + &c_args, )?); }