diff --git a/bindgen/clang.rs b/bindgen/clang.rs index 9cb5a03d28..4425f51ca7 100644 --- a/bindgen/clang.rs +++ b/bindgen/clang.rs @@ -8,6 +8,7 @@ use crate::ir::context::{BindgenContext, IncludeLocation}; use clang_sys::*; use std::cmp; +use std::convert::TryInto; use std::ffi::{CStr, CString}; use std::fmt; use std::hash::Hash; @@ -135,8 +136,7 @@ impl Cursor { /// Returns whether the cursor refers to a built-in definition. pub(crate) fn is_builtin(&self) -> bool { - let (file, _, _, _) = self.location().location(); - file.name().is_none() + matches!(self.location(), SourceLocation::Builtin { .. }) } /// Get the `Cursor` for this cursor's referent's lexical parent. @@ -376,8 +376,31 @@ impl Cursor { /// Get the source location for the referent. pub(crate) fn location(&self) -> SourceLocation { unsafe { - SourceLocation { - x: clang_getCursorLocation(self.x), + let location = clang_getCursorLocation(self.x); + + let mut file = mem::zeroed(); + let mut line = 0; + let mut column = 0; + let mut offset = 0; + clang_getSpellingLocation( + location, + &mut file, + &mut line, + &mut column, + &mut offset, + ); + + if file.is_null() { + SourceLocation::Builtin { + offset: offset.try_into().unwrap(), + } + } else { + SourceLocation::File { + file_name: cxstring_into_string(clang_getFileName(file)), + line: line.try_into().unwrap(), + column: column.try_into().unwrap(), + offset: offset.try_into().unwrap(), + } } } } @@ -512,11 +535,7 @@ impl Cursor { for child in &children { if child.kind() == CXCursor_InclusionDirective { if let Some(included_file) = child.get_included_file_name() { - let location = child.location(); - let (source_file, _, _, offset) = location.location(); - - let source_file = source_file.name(); - ctx.add_include(source_file, included_file, offset); + ctx.add_include(included_file, child.location()); } } } @@ -527,171 +546,16 @@ impl Cursor { } } - /// Compare source order of two cursors, considering `#include` directives. - /// - /// Built-in items provided by the compiler (which don't have a source file), - /// are sorted first. Remaining files are sorted by their position in the source file. - /// If the items' source files differ, they are sorted by the position of the first - /// `#include` for their source file. If no source files are included, `None` is returned. + /// Compare two cursors according how they appear in source files. fn cmp_by_source_order( &self, other: &Self, ctx: &BindgenContext, ) -> cmp::Ordering { - let (file, _, _, offset) = self.location().location(); - let (other_file, _, _, other_offset) = other.location().location(); - - let (file, other_file) = match (file.name(), other_file.name()) { - (Some(file), Some(other_file)) => (file, other_file), - // Keep the original sorting of built-in definitions. - (None, _) | (_, None) => return cmp::Ordering::Equal, - }; - - // If both items are in the same source file, simply compare the offset. - if file == other_file { - return offset.cmp(&other_offset); - } - - let mut include_location = ctx.included_file_location(&file); - let mut other_include_location = - ctx.included_file_location(&other_file); - - use IncludeLocation::*; - - loop { - match (&include_location, &other_include_location) { - // Both items are in the main header file, this should already have been handled at this point. - (Main, Main) => { - unreachable!("Should have been handled at this point.") - } - // Headers passed as CLI arguments come before the main header file. - (Main, Cli { .. }) => return cmp::Ordering::Greater, - (Cli { .. }, Main) => return cmp::Ordering::Less, - // If both were included via CLI arguments, compare their offset. - ( - Cli { offset: offset2 }, - Cli { - offset: other_offset2, - }, - ) => return offset2.cmp(other_offset2), - // If an item was included in the same source file as the other item, - // compare its `#include` location offset the offset of the other item. - ( - File { - file_name: ref file2, - offset: offset2, - }, - Main, - ) => { - if *file2 == other_file { - return offset2.cmp(&other_offset); - } + let location = self.location(); + let other_location = other.location(); - // Continue checking one level up. - include_location = ctx.included_file_location(file2); - } - ( - Main, - File { - file_name: ref other_file2, - offset: other_offset2, - }, - ) => { - if file == *other_file2 { - return offset.cmp(other_offset2); - } - - // Continue checking one level up. - other_include_location = - ctx.included_file_location(other_file2); - } - ( - File { - file_name: file2, - offset: offset2, - }, - File { - file_name: other_file2, - offset: other_offset2, - }, - ) => { - // If both items were included in the same file, compare the offset of their `#include` directives. - if file2 == other_file2 { - return offset2.cmp(other_offset2); - } - - // Find the offset of where `file` is transivitely included in `ancestor_file`. - let offset_in_ancestor = - |mut file: String, ancestor_file: &str| { - while file != ancestor_file { - let include_location = - ctx.included_file_location(&file); - file = if let IncludeLocation::File { - file_name: file, - offset, - } = include_location - { - if file == ancestor_file { - return Some(offset); - } - - file - } else { - break; - } - } - - None - }; - - if let Some(offset2) = - offset_in_ancestor(file2.clone(), other_file2) - { - return offset2.cmp(other_offset2); - } - - if let Some(other_offset2) = - offset_in_ancestor(other_file2.clone(), file2) - { - return offset2.cmp(&other_offset2); - } - - // Otherwise, go one level up. - // - // # Example - // - // a.h - // ├── b.h - // └── c.h - // - // When comparing items inside `b.h` and `c.h`, go up one level and - // compare the include locations of `b.h` and `c.h` in `a.h` instead. - include_location = ctx.included_file_location(file2); - other_include_location = - ctx.included_file_location(other_file2); - } - ( - File { - file_name: file2, .. - }, - Cli { .. }, - ) => { - // Continue checking one level up. - include_location = ctx.included_file_location(file2); - } - ( - Cli { .. }, - File { - file_name: other_file2, - .. - }, - ) => { - // Continue checking one level up. - other_include_location = - ctx.included_file_location(other_file2); - } - } - } + location.cmp_by_source_order(&other_location, ctx) } /// Collect all of this cursor's children into a vec and return them. @@ -1699,36 +1563,120 @@ impl ExactSizeIterator for TypeTemplateArgIterator { } } -/// A `SourceLocation` is a file, line, column, and byte offset location for -/// some source text. -pub(crate) struct SourceLocation { - x: CXSourceLocation, +/// A location of some source code. +#[derive(Clone)] +pub(crate) enum SourceLocation { + /// Location of a built-in. + Builtin { + /// Offset. + offset: usize, + }, + /// Location in a source file. + File { + /// Name of the source file. + file_name: String, + /// Line in the source file. + line: usize, + /// Column in the source file. + column: usize, + /// Offset in the source file. + offset: usize, + }, } impl SourceLocation { - /// Get the (file, line, column, byte offset) tuple for this source - /// location. - pub(crate) fn location(&self) -> (File, usize, usize, usize) { - unsafe { - let mut file = mem::zeroed(); - let mut line = 0; - let mut col = 0; - let mut off = 0; - clang_getSpellingLocation( - self.x, &mut file, &mut line, &mut col, &mut off, - ); - (File { x: file }, line as usize, col as usize, off as usize) + /// 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. + /// If the locations' source files differ, they are sorted by the position of where the + /// source files are first transitively included, see `IncludeLocation::cmp_by_source_order`. + pub fn cmp_by_source_order( + &self, + other: &Self, + ctx: &BindgenContext, + ) -> cmp::Ordering { + match (self, other) { + ( + SourceLocation::Builtin { offset }, + SourceLocation::Builtin { + offset: other_offset, + }, + ) => offset.cmp(other_offset), + // Built-ins come first. + (SourceLocation::Builtin { .. }, _) => cmp::Ordering::Less, + (_, SourceLocation::Builtin { .. }) => { + other.cmp_by_source_order(self, ctx).reverse() + } + ( + SourceLocation::File { + file_name, offset, .. + }, + SourceLocation::File { + file_name: other_file_name, + offset: other_offset, + .. + }, + ) => { + if file_name == other_file_name { + 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: &str, ancestor_file: &str| { + let mut file = file.to_owned(); + while file != ancestor_file { + let include_location = + ctx.included_file_location(&file); + file = if let IncludeLocation::File { + file_name: file, + offset, + .. + } = include_location + { + if file == ancestor_file { + return Some(offset); + } + + file + } else { + break; + } + } + + None + }; + + if let Some(offset) = + offset_in_ancestor(file_name, other_file_name) + { + return offset.cmp(other_offset); + } + + if let Some(other_offset) = + offset_in_ancestor(other_file_name, file_name) + { + return offset.cmp(&other_offset); + } + + // If the source files are siblings, compare their include locations. + let parent = ctx.included_file_location(file_name); + let other_parent = ctx.included_file_location(other_file_name); + parent.cmp_by_source_order(&other_parent, ctx) + } } } } impl fmt::Display for SourceLocation { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let (file, line, col, _) = self.location(); - if let Some(name) = file.name() { - write!(f, "{}:{}:{}", name, line, col) - } else { - "builtin definitions".fmt(f) + match self { + Self::Builtin { .. } => "built-in".fmt(f), + Self::File { + file_name, + line, + column, + .. + } => write!(f, "{}:{}:{}", file_name, line, column), } } } @@ -1838,21 +1786,6 @@ impl Iterator for CommentAttributesIterator { } } -/// A source file. -pub(crate) struct File { - x: CXFile, -} - -impl File { - /// Get the name of this source file. - pub(crate) fn name(&self) -> Option { - if self.x.is_null() { - return None; - } - Some(unsafe { cxstring_into_string(clang_getFileName(self.x)) }) - } -} - fn cxstring_to_string_leaky(s: CXString) -> String { if s.data.is_null() { return "".to_owned(); diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 11425e02a4..f203daaa53 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -4353,17 +4353,19 @@ fn unsupported_abi_diagnostic( Level::Note, ); - if let Some(loc) = location { - let (file, line, col, _) = loc.location(); - - if let Some(filename) = file.name() { - if let Ok(Some(source)) = get_line(&filename, line) { - let mut slice = Slice::default(); - slice - .with_source(source) - .with_location(filename, line, col); - diag.add_slice(slice); - } + if let Some(crate::clang::SourceLocation::File { + file_name, + line, + column, + .. + }) = location.cloned() + { + if let Ok(Some(source)) = get_line(&file_name, line) { + let mut slice = Slice::default(); + slice + .with_source(source) + .with_location(file_name, line, column); + diag.add_slice(slice); } } @@ -4371,10 +4373,11 @@ fn unsupported_abi_diagnostic( } } +#[cfg_attr(not(feature = "experimental"), allow(unused_variables))] fn variadic_fn_diagnostic( fn_name: &str, - _location: Option<&crate::clang::SourceLocation>, - _ctx: &BindgenContext, + location: Option<&crate::clang::SourceLocation>, + ctx: &BindgenContext, ) { warn!( "Cannot generate wrapper for the static variadic function `{}`.", @@ -4382,7 +4385,7 @@ fn variadic_fn_diagnostic( ); #[cfg(feature = "experimental")] - if _ctx.options().emit_diagnostics { + if ctx.options().emit_diagnostics { use crate::diagnostics::{get_line, Diagnostic, Level, Slice}; let mut diag = Diagnostic::default(); @@ -4391,17 +4394,19 @@ fn variadic_fn_diagnostic( .add_annotation("The `--wrap-static-fns` feature does not support variadic functions.", Level::Note) .add_annotation("No code will be generated for this function.", Level::Note); - if let Some(loc) = _location { - let (file, line, col, _) = loc.location(); - - if let Some(filename) = file.name() { - if let Ok(Some(source)) = get_line(&filename, line) { - let mut slice = Slice::default(); - slice - .with_source(source) - .with_location(filename, line, col); - diag.add_slice(slice); - } + if let Some(crate::clang::SourceLocation::File { + file_name, + line, + column, + .. + }) = location.cloned() + { + if let Ok(Some(source)) = get_line(&file_name, line) { + let mut slice = Slice::default(); + slice + .with_source(source) + .with_location(file_name, line, column); + diag.add_slice(slice); } } diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index d30727abb4..8f45474aba 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, Cursor}; +use crate::clang::{self, Cursor, SourceLocation}; use crate::codegen::CodegenError; use crate::BindgenOptions; use crate::{Entry, HashMap, HashSet}; @@ -30,7 +30,7 @@ use std::borrow::Cow; use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap as StdHashMap}; use std::iter::IntoIterator; -use std::mem; +use std::{cmp, mem}; /// An identifier for some kind of IR item. #[derive(Debug, Copy, Clone, Eq, PartialOrd, Ord, Hash)] @@ -319,11 +319,95 @@ pub(crate) enum IncludeLocation { File { /// Source file name of the include location. file_name: String, + /// Line of the include location. + line: usize, + /// Column of the include location. + column: usize, /// Offset of the include location. offset: usize, }, } +impl IncludeLocation { + /// Header include locations are sorted in the order they appear in, which means + /// headers provided as command line arguments (`-include`) are sorted first, + /// followed by the main header file, and lastly headers included with `#include` + /// directives. + /// + /// Nested headers are sorted according to where they were transitively included first. + pub fn cmp_by_source_order( + &self, + other: &Self, + ctx: &BindgenContext, + ) -> cmp::Ordering { + match (self, other) { + // If both headers are included with CLI argument, compare their offset. + ( + IncludeLocation::Cli { offset }, + IncludeLocation::Cli { + offset: other_offset, + }, + ) => offset.cmp(other_offset), + // Headers included with CLI arguments come first. + (IncludeLocation::Cli { .. }, IncludeLocation::Main) => { + cmp::Ordering::Less + } + (IncludeLocation::Main, IncludeLocation::Cli { .. }) => { + cmp::Ordering::Greater + } + // If both includes refer to the main header file, they are equal. + (IncludeLocation::Main, IncludeLocation::Main) => { + cmp::Ordering::Equal + } + // If one is included via CLI arguments or the main header file, + // recursively compare with the other's include location. + ( + IncludeLocation::Cli { .. } | IncludeLocation::Main, + IncludeLocation::File { + file_name: other_file_name, + .. + }, + ) => { + let other_parent: IncludeLocation = + ctx.included_file_location(other_file_name); + self.cmp_by_source_order(&other_parent, ctx) + } + ( + IncludeLocation::File { .. }, + IncludeLocation::Cli { .. } | IncludeLocation::Main, + ) => other.cmp_by_source_order(self, ctx).reverse(), + ( + IncludeLocation::File { + file_name, + line, + column, + offset, + }, + IncludeLocation::File { + file_name: other_file_name, + line: other_line, + column: other_column, + offset: other_offset, + }, + ) => SourceLocation::File { + file_name: file_name.clone(), + line: *line, + column: *column, + offset: *offset, + } + .cmp_by_source_order( + &SourceLocation::File { + file_name: other_file_name.clone(), + line: *other_line, + column: *other_column, + offset: *other_offset, + }, + ctx, + ), + } + } +} + /// A context used during parsing and generation of structs. #[derive(Debug)] pub(crate) struct BindgenContext { @@ -381,7 +465,7 @@ pub(crate) struct BindgenContext { /// /// The key is the included file, the value is a pair of the source file and /// the position of the `#include` directive in the source file. - includes: StdHashMap, usize)>, + includes: StdHashMap, /// A set of all the included filenames. deps: BTreeSet, @@ -665,13 +749,12 @@ If you encounter an error missing from this list, please file an issue or a PR!" /// Add the location of the `#include` directive for the `included_file`. pub(crate) fn add_include( &mut self, - source_file: Option, included_file: String, - offset: usize, + source_location: SourceLocation, ) { self.includes .entry(included_file) - .or_insert((source_file, offset)); + .or_insert(source_location); } /// Get the location of the first `#include` directive for the `included_file`. @@ -682,12 +765,22 @@ If you encounter an error missing from this list, please file an issue or a PR!" match self.includes.get(included_file).cloned() { // Header was not included anywhere, so it must be the main header. None => IncludeLocation::Main, - // Header has no source location, so it must have been included via CLI arguments. - Some((None, offset)) => IncludeLocation::Cli { offset }, - // Header was included with an `#include` directive. - Some((Some(file_name), offset)) => { - IncludeLocation::File { file_name, offset } + // Include location is a builtin, so it must have been included via CLI arguments. + Some(SourceLocation::Builtin { offset }) => { + IncludeLocation::Cli { offset } } + // Header was included with an `#include` directive. + Some(SourceLocation::File { + file_name, + line, + column, + offset, + }) => IncludeLocation::File { + file_name, + line, + column, + offset, + }, } } @@ -2385,16 +2478,16 @@ 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(location) = item.location() { - let (file, _, _, _) = location.location(); - if let Some(filename) = file.name() { - if self - .options() - .allowlisted_files - .matches(filename) - { - return true; - } + if let Some(SourceLocation::File { + file_name, .. + }) = item.location() + { + if self + .options() + .allowlisted_files + .matches(file_name) + { + return true; } } } diff --git a/bindgen/ir/item.rs b/bindgen/ir/item.rs index 1b9d7694b2..52df62c046 100644 --- a/bindgen/ir/item.rs +++ b/bindgen/ir/item.rs @@ -17,7 +17,7 @@ use super::module::Module; use super::template::{AsTemplateParam, TemplateParameters}; use super::traversal::{EdgeKind, Trace, Tracer}; use super::ty::{Type, TypeKind}; -use crate::clang; +use crate::clang::{self, SourceLocation}; use crate::parse::{ClangSubItemParser, ParseError, ParseResult}; use lazycell::LazyCell; @@ -647,12 +647,10 @@ impl Item { } if !ctx.options().blocklisted_files.is_empty() { - if let Some(location) = &self.location { - let (file, _, _, _) = location.location(); - if let Some(filename) = file.name() { - if ctx.options().blocklisted_files.matches(filename) { - return true; - } + if let Some(SourceLocation::File { file_name, .. }) = &self.location + { + if ctx.options().blocklisted_files.matches(file_name) { + return true; } } } diff --git a/bindgen/ir/var.rs b/bindgen/ir/var.rs index a548ec881b..6bbe4a9acc 100644 --- a/bindgen/ir/var.rs +++ b/bindgen/ir/var.rs @@ -441,10 +441,11 @@ fn get_integer_literal_from_cursor(cursor: &clang::Cursor) -> Option { value } +#[cfg_attr(not(feature = "experimental"), allow(unused_variables))] fn duplicated_macro_diagnostic( macro_name: &str, - _location: crate::clang::SourceLocation, - _ctx: &BindgenContext, + location: crate::clang::SourceLocation, + ctx: &BindgenContext, ) { warn!("Duplicated macro definition: {}", macro_name); @@ -462,19 +463,24 @@ fn duplicated_macro_diagnostic( // // Will trigger this message even though there's nothing wrong with it. #[allow(clippy::overly_complex_bool_expr)] - if false && _ctx.options().emit_diagnostics { + if false && ctx.options().emit_diagnostics { use crate::diagnostics::{get_line, Diagnostic, Level, Slice}; use std::borrow::Cow; let mut slice = Slice::default(); let mut source = Cow::from(macro_name); - let (file, line, col, _) = _location.location(); - if let Some(filename) = file.name() { - if let Ok(Some(code)) = get_line(&filename, line) { + if let crate::clang::SourceLocation::File { + file_name, + line, + column, + .. + } = location + { + if let Ok(Some(code)) = get_line(&file_name, line) { source = code.into(); } - slice.with_location(filename, line, col); + slice.with_location(file_name, line, column); } slice.with_source(source);