diff --git a/bindgen/clang.rs b/bindgen/clang.rs index 179dbc8f03..e4e36a6db9 100644 --- a/bindgen/clang.rs +++ b/bindgen/clang.rs @@ -6,8 +6,10 @@ use crate::ir::context::{BindgenContext, IncludeLocation}; use clang_sys::*; +use std::borrow::Cow; use std::cmp; use std::convert::TryInto; +use std::env::current_dir; use std::ffi::{CStr, CString}; use std::fmt; use std::hash::Hash; @@ -396,9 +398,8 @@ impl Cursor { offset: offset.try_into().unwrap(), } } else { - let file_name = cxstring_into_string(clang_getFileName(file)); SourceLocation::File { - file_path: absolutize_path(file_name), + file: SourceFile::from_raw(file), line: line.try_into().unwrap(), column: column.try_into().unwrap(), offset: offset.try_into().unwrap(), @@ -536,12 +537,8 @@ impl Cursor { let mut children = self.collect_children(); for child in &children { if child.kind() == CXCursor_InclusionDirective { - if let Some(included_file_name) = child.get_included_file_name() - { - ctx.add_include( - absolutize_path(included_file_name), - child.location(), - ); + if let Some(included_file) = child.get_included_file() { + ctx.add_include(included_file, child.location()); } } } @@ -934,14 +931,12 @@ impl Cursor { /// Obtain the real path name of a cursor of InclusionDirective kind. /// /// Returns None if the cursor does not include a file, otherwise the file's full name - pub(crate) fn get_included_file_name(&self) -> Option { - let file = unsafe { clang_sys::clang_getIncludedFile(self.x) }; + pub(crate) fn get_included_file(&self) -> Option { + let file = unsafe { clang_getIncludedFile(self.x) }; if file.is_null() { None } else { - Some(unsafe { - cxstring_into_string(clang_sys::clang_getFileName(file)) - }) + Some(unsafe { SourceFile::from_raw(file) }) } } } @@ -1579,8 +1574,8 @@ pub(crate) enum SourceLocation { }, /// Location in a source file. File { - /// Name of the source file. - file_path: PathBuf, + /// The source file. + file: SourceFile, /// Line in the source file. line: usize, /// Column in the source file. @@ -1590,18 +1585,6 @@ pub(crate) enum SourceLocation { }, } -fn absolutize_path>(path: P) -> PathBuf { - let path = path.as_ref(); - - if path.is_relative() { - std::env::current_dir() - .expect("Cannot retrieve current directory") - .join(path) - } else { - path.to_owned() - } -} - impl SourceLocation { /// Locations of built-in items provided by the compiler (which don't have a source file), /// are sorted first. Remaining locations are sorted by their position in the source file. @@ -1625,59 +1608,64 @@ impl SourceLocation { other.cmp_by_source_order(self, ctx).reverse() } ( + SourceLocation::File { file, offset, .. }, SourceLocation::File { - file_path, offset, .. - }, - SourceLocation::File { - file_path: other_file_path, + file: other_file, offset: other_offset, .. }, ) => { + let file_path = file.path(); + let other_file_path = other_file.path(); + if file_path == other_file_path { return offset.cmp(other_offset); } // If `file` is transitively included via `ancestor_file`, // find the offset of the include directive in `ancestor_file`. - let offset_in_ancestor = |file: &Path, ancestor_file: &Path| { - let mut file = file; - while file != ancestor_file { - let include_location = ctx.include_location(file); - file = if let IncludeLocation::File { - file_path: file, - offset, - .. - } = include_location - { - if file == ancestor_file { - return Some(offset); + let offset_in_ancestor = + |file_path: &Path, ancestor_file_path: &Path| { + let mut file_path = Cow::Borrowed(file_path); + while file_path != ancestor_file_path { + let include_location = + ctx.include_location(file_path); + file_path = if let IncludeLocation::File { + file, + offset, + .. + } = include_location + { + let file_path = Cow::Owned(file.path()); + + if file_path == ancestor_file_path { + return Some(offset); + } + + file_path + } else { + break; } - - file - } else { - break; } - } - None - }; + None + }; if let Some(offset) = - offset_in_ancestor(file_path, other_file_path) + offset_in_ancestor(&file_path, &other_file_path) { return offset.cmp(other_offset); } if let Some(other_offset) = - offset_in_ancestor(other_file_path, file_path) + offset_in_ancestor(&other_file_path, &file_path) { return offset.cmp(other_offset); } // If the source files are siblings, compare their include locations. let parent = ctx.include_location(file_path); - let other_parent = ctx.include_location(other_file_path); + let other_parent = ctx.include_location(&other_file_path); parent.cmp_by_source_order(other_parent, ctx) } } @@ -1689,11 +1677,8 @@ impl fmt::Display for SourceLocation { match self { Self::Builtin { .. } => "built-in".fmt(f), Self::File { - file_path, - line, - column, - .. - } => write!(f, "{}:{}:{}", file_path.display(), line, column), + file, line, column, .. + } => write!(f, "{}:{}:{}", file.path().display(), line, column), } } } @@ -1803,17 +1788,63 @@ impl Iterator for CommentAttributesIterator { } } -fn cxstring_to_string_leaky(s: CXString) -> String { +/// A source file. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct SourceFile { + name: Box, +} + +impl SourceFile { + /// Creates a new `SourceFile` from a file name. + #[inline] + pub fn new>(name: P) -> Self { + Self { + name: name.as_ref().to_owned().into_boxed_str(), + } + } + + /// Creates a new `SourceFile` from a raw pointer. + /// + /// # Safety + /// + /// `file` must point to a valid `CXFile`. + pub unsafe fn from_raw(file: CXFile) -> Self { + let name = unsafe { cxstring_into_string(clang_getFileName(file)) }; + + Self::new(name) + } + + #[inline] + pub fn name(&self) -> &str { + &self.name + } + + /// Get the path of this source file. + pub fn path(&self) -> PathBuf { + let path = Path::new(self.name()); + if path.is_relative() { + current_dir() + .expect("Cannot retrieve current directory") + .join(path) + } else { + path.to_owned() + } + } +} + +unsafe fn cxstring_to_string_leaky(s: CXString) -> String { if s.data.is_null() { - return "".to_owned(); + Cow::Borrowed("") + } else { + let c_str = CStr::from_ptr(clang_getCString(s) as *const _); + c_str.to_string_lossy() } - let c_str = unsafe { CStr::from_ptr(clang_getCString(s) as *const _) }; - c_str.to_string_lossy().into_owned() + .into_owned() } -fn cxstring_into_string(s: CXString) -> String { +unsafe fn cxstring_into_string(s: CXString) -> String { let ret = cxstring_to_string_leaky(s); - unsafe { clang_disposeString(s) }; + clang_disposeString(s); ret } @@ -1924,13 +1955,13 @@ impl TranslationUnit { } } - /// Get the source file path of this translation unit. - pub(crate) fn path(&self) -> PathBuf { - let file_name = unsafe { + /// Get the source file of this. + pub fn file(&self) -> SourceFile { + let name = unsafe { cxstring_into_string(clang_getTranslationUnitSpelling(self.x)) }; - absolutize_path(file_name) + SourceFile::new(name) } /// Is this the null translation unit? diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 7ac8aff9aa..bd86510cc7 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -4379,12 +4379,13 @@ fn unsupported_abi_diagnostic( ); if let Some(crate::clang::SourceLocation::File { - file_path, + file, line, column, .. }) = location.cloned() { + let file_path = file.path(); if let Ok(Some(source)) = get_line(&file_path, line) { let mut slice = Slice::default(); slice @@ -4420,12 +4421,13 @@ fn variadic_fn_diagnostic( .add_annotation("No code will be generated for this function.", Level::Note); if let Some(crate::clang::SourceLocation::File { - file_path, + file, line, column, .. }) = location.cloned() { + let file_path = file.path(); if let Ok(Some(source)) = get_line(&file_path, line) { let mut slice = Slice::default(); slice diff --git a/bindgen/deps.rs b/bindgen/deps.rs index be31f92896..6d26b537d2 100644 --- a/bindgen/deps.rs +++ b/bindgen/deps.rs @@ -1,6 +1,8 @@ /// Generating build depfiles from parsed bindings. use std::{collections::BTreeSet, path::PathBuf}; +use crate::clang::SourceFile; + #[derive(Clone, Debug)] pub(crate) struct DepfileSpec { pub output_module: String, @@ -8,17 +10,17 @@ pub(crate) struct DepfileSpec { } impl DepfileSpec { - pub fn write(&self, deps: &BTreeSet>) -> std::io::Result<()> { + pub fn write(&self, deps: &BTreeSet) -> std::io::Result<()> { std::fs::write(&self.depfile_path, self.to_string(deps)) } - fn to_string(&self, deps: &BTreeSet>) -> String { + fn to_string(&self, deps: &BTreeSet) -> String { // Transforms a string by escaping spaces and backslashes. let escape = |s: &str| s.replace('\\', "\\\\").replace(' ', "\\ "); let mut buf = format!("{}:", escape(&self.output_module)); for file in deps { - buf = format!("{} {}", buf, escape(file)); + buf = format!("{} {}", buf, escape(file.name())); } buf } @@ -36,13 +38,13 @@ mod tests { }; let deps: BTreeSet<_> = vec![ - r"/absolute/path".into(), - r"C:\win\absolute\path".into(), - r"../relative/path".into(), - r"..\win\relative\path".into(), - r"../path/with spaces/in/it".into(), - r"..\win\path\with spaces\in\it".into(), - r"path\with/mixed\separators".into(), + SourceFile::new(r"/absolute/path"), + SourceFile::new(r"C:\win\absolute\path"), + SourceFile::new(r"../relative/path"), + SourceFile::new(r"..\win\relative\path"), + SourceFile::new(r"../path/with spaces/in/it"), + SourceFile::new(r"..\win\path\with spaces\in\it"), + SourceFile::new(r"path\with/mixed\separators"), ] .into_iter() .collect(); diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 4d31f4bd4d..975ab1e1d5 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -19,7 +19,7 @@ use super::module::{Module, ModuleKind}; use super::template::{TemplateInstantiation, TemplateParameters}; use super::traversal::{self, Edge, ItemTraversal}; use super::ty::{FloatKind, Type, TypeKind}; -use crate::clang::{self, ABIKind, Cursor, SourceLocation}; +use crate::clang::{self, ABIKind, Cursor, SourceFile, SourceLocation}; use crate::codegen::CodegenError; use crate::BindgenOptions; use crate::{Entry, HashMap, HashSet}; @@ -320,7 +320,7 @@ pub(crate) enum IncludeLocation { /// Include location of a header included with an `#include` directive. File { /// Source file name of the include location. - file_path: PathBuf, + file: SourceFile, /// Line of the include location. line: usize, /// Column of the include location. @@ -366,11 +366,10 @@ impl IncludeLocation { ( IncludeLocation::Cli { .. } | IncludeLocation::Main, IncludeLocation::File { - file_path: other_file_path, - .. + file: other_file, .. }, ) => { - let other_parent = ctx.include_location(other_file_path); + let other_parent = ctx.include_location(other_file.path()); self.cmp_by_source_order(other_parent, ctx) } ( @@ -380,26 +379,26 @@ impl IncludeLocation { // If both include locations are in files, compare them as `SourceLocation`s. ( IncludeLocation::File { - file_path, + file, line, column, offset, }, IncludeLocation::File { - file_path: other_file_path, + file: other_file, line: other_line, column: other_column, offset: other_offset, }, ) => SourceLocation::File { - file_path: file_path.clone(), + file: file.clone(), line: *line, column: *column, offset: *offset, } .cmp_by_source_order( &SourceLocation::File { - file_path: other_file_path.clone(), + file: other_file.clone(), line: *other_line, column: *other_column, offset: *other_offset, @@ -470,7 +469,7 @@ pub(crate) struct BindgenContext { includes: StdHashMap, /// A set of all the included filenames. - deps: BTreeSet>, + deps: BTreeSet, /// The active replacements collected from replaces="xxx" annotations. replacements: HashMap, ItemId>, @@ -672,7 +671,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" let root_module_id = root_module.id().as_module_id_unchecked(); // depfiles need to include the explicitly listed headers too - let deps = options.input_headers.iter().cloned().collect(); + let deps = options.input_headers.iter().map(SourceFile::new).collect(); BindgenContext { items: vec![Some(root_module)], @@ -760,16 +759,18 @@ If you encounter an error missing from this list, please file an issue or a PR!" /// Add the location of where `included_file` is included. pub(crate) fn add_include( &mut self, - included_file_path: PathBuf, + included_file: SourceFile, source_location: SourceLocation, ) { + let included_file_path = included_file.path(); + if self.includes.contains_key(&included_file_path) { return; } let include_location = { // Recursively including the main header file doesn't count. - if self.translation_unit.path() == included_file_path { + if self.translation_unit.file().path() == included_file_path { IncludeLocation::Main } else { match source_location { @@ -779,12 +780,12 @@ If you encounter an error missing from this list, please file an issue or a PR!" } // Header was included with an `#include` directive. SourceLocation::File { - file_path, + file, line, column, offset, } => IncludeLocation::File { - file_path, + file, line, column, offset, @@ -808,12 +809,12 @@ If you encounter an error missing from this list, please file an issue or a PR!" } /// Add an included file. - pub(crate) fn add_dep(&mut self, dep: Box) { + pub(crate) fn add_dep(&mut self, dep: SourceFile) { self.deps.insert(dep); } /// Get any included files. - pub(crate) fn deps(&self) -> &BTreeSet> { + pub(crate) fn deps(&self) -> &BTreeSet { &self.deps } @@ -2504,14 +2505,13 @@ If you encounter an error missing from this list, please file an issue or a PR!" // Items with a source location in an explicitly allowlisted file // are always included. if !self.options().allowlisted_files.is_empty() { - if let Some(SourceLocation::File { - file_path, .. - }) = item.location() + if let Some(SourceLocation::File { file, .. }) = + item.location() { if self .options() .allowlisted_files - .matches(file_path.display().to_string()) + .matches(file.path().display().to_string()) { return true; } diff --git a/bindgen/ir/item.rs b/bindgen/ir/item.rs index 9a035ff6a1..669cd7e542 100644 --- a/bindgen/ir/item.rs +++ b/bindgen/ir/item.rs @@ -641,12 +641,11 @@ impl Item { } if !ctx.options().blocklisted_files.is_empty() { - if let Some(SourceLocation::File { file_path, .. }) = &self.location - { + if let Some(SourceLocation::File { file, .. }) = &self.location { if ctx .options() .blocklisted_files - .matches(file_path.display().to_string()) + .matches(file.path().display().to_string()) { return true; } @@ -1432,17 +1431,17 @@ impl Item { } CXCursor_InclusionDirective => { - let file = cursor.get_included_file_name(); + let file = cursor.get_included_file(); match file { - None => { - warn!("Inclusion of a nameless file in {:?}", cursor); - } Some(included_file) => { for cb in &ctx.options().parse_callbacks { - cb.include_file(&included_file); + cb.include_file(included_file.name()); } - ctx.add_dep(included_file.into_boxed_str()); + ctx.add_dep(included_file); + } + None => { + warn!("Inclusion of a nameless file in {:?}", cursor) } } Err(ParseError::Continue) diff --git a/bindgen/ir/var.rs b/bindgen/ir/var.rs index 011a83e034..950eeeda93 100644 --- a/bindgen/ir/var.rs +++ b/bindgen/ir/var.rs @@ -471,12 +471,10 @@ fn duplicated_macro_diagnostic( let mut source = Cow::from(macro_name); if let crate::clang::SourceLocation::File { - file_path, - line, - column, - .. + file, line, column, .. } = location { + let file_path = file.path(); if let Ok(Some(code)) = get_line(&file_path, line) { source = code.into(); }