Skip to content

Commit

Permalink
Don't count include locations of main header file.
Browse files Browse the repository at this point in the history
  • Loading branch information
reitermarkus committed Jun 21, 2023
1 parent 6553527 commit 909d926
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 70 deletions.
67 changes: 67 additions & 0 deletions bindgen-tests/tests/expectations/tests/source-order-recursive.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions bindgen-tests/tests/headers/source-order-recursive.h
@@ -0,0 +1,14 @@
// bindgen-flags: -- -Itests/headers -Itests/headers/source-order-recursive

#ifndef A_H
#define A_H

struct foo {};

#include "source-order-recursive-2.h"

struct baz {
struct bar field;
};

#endif
@@ -0,0 +1,5 @@
#include "source-order-recursive.h"

struct bar {
struct foo field;
};
59 changes: 43 additions & 16 deletions bindgen/clang.rs
Expand Up @@ -7,13 +7,14 @@
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;
use std::hash::Hasher;
use std::os::raw::{c_char, c_int, c_longlong, c_uint, c_ulong, c_ulonglong};
use std::path::Path;
use std::path::PathBuf;
use std::{mem, ptr, slice};

/// Type representing a clang attribute.
Expand Down Expand Up @@ -395,8 +396,9 @@ impl Cursor {
offset: offset.try_into().unwrap(),
}
} else {
let file_name = cxstring_into_string(clang_getFileName(file));
SourceLocation::File {
file_name: cxstring_into_string(clang_getFileName(file)),
file_path: absolutize_path(file_name),
line: line.try_into().unwrap(),
column: column.try_into().unwrap(),
offset: offset.try_into().unwrap(),
Expand Down Expand Up @@ -534,8 +536,12 @@ impl Cursor {
let mut children = self.collect_children();
for child in &children {
if child.kind() == CXCursor_InclusionDirective {
if let Some(included_file) = child.get_included_file_name() {
ctx.add_include(included_file, child.location());
if let Some(included_file_name) = child.get_included_file_name()
{
ctx.add_include(
absolutize_path(included_file_name),
child.location(),
);
}
}
}
Expand Down Expand Up @@ -1574,7 +1580,7 @@ pub(crate) enum SourceLocation {
/// Location in a source file.
File {
/// Name of the source file.
file_name: String,
file_path: PathBuf,
/// Line in the source file.
line: usize,
/// Column in the source file.
Expand All @@ -1584,6 +1590,18 @@ pub(crate) enum SourceLocation {
},
}

fn absolutize_path<P: AsRef<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.
Expand All @@ -1608,26 +1626,26 @@ impl SourceLocation {
}
(
SourceLocation::File {
file_name, offset, ..
file_path, offset, ..
},
SourceLocation::File {
file_name: other_file_name,
file_path: other_file_path,
offset: other_offset,
..
},
) => {
if file_name == other_file_name {
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: &str, ancestor_file: &str| {
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_name: file,
file_path: file,
offset,
..
} = include_location
Expand All @@ -1646,20 +1664,20 @@ impl SourceLocation {
};

if let Some(offset) =
offset_in_ancestor(file_name, other_file_name)
offset_in_ancestor(file_path, other_file_path)
{
return offset.cmp(other_offset);
}

if let Some(other_offset) =
offset_in_ancestor(other_file_name, file_name)
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_name);
let other_parent = ctx.include_location(other_file_name);
let parent = ctx.include_location(file_path);
let other_parent = ctx.include_location(other_file_path);
parent.cmp_by_source_order(other_parent, ctx)
}
}
Expand All @@ -1671,11 +1689,11 @@ impl fmt::Display for SourceLocation {
match self {
Self::Builtin { .. } => "built-in".fmt(f),
Self::File {
file_name,
file_path,
line,
column,
..
} => write!(f, "{}:{}:{}", file_name, line, column),
} => write!(f, "{}:{}:{}", file_path.display(), line, column),
}
}
}
Expand Down Expand Up @@ -1906,6 +1924,15 @@ impl TranslationUnit {
}
}

/// Get the source file path of this translation unit.
pub(crate) fn path(&self) -> PathBuf {
let file_name = unsafe {
cxstring_into_string(clang_getTranslationUnitSpelling(self.x))
};

absolutize_path(file_name)
}

/// Is this the null translation unit?
pub(crate) fn is_null(&self) -> bool {
self.x.is_null()
Expand Down
12 changes: 6 additions & 6 deletions bindgen/codegen/mod.rs
Expand Up @@ -4354,17 +4354,17 @@ fn unsupported_abi_diagnostic(
);

if let Some(crate::clang::SourceLocation::File {
file_name,
file_path,
line,
column,
..
}) = location.cloned()
{
if let Ok(Some(source)) = get_line(&file_name, line) {
if let Ok(Some(source)) = get_line(&file_path, line) {
let mut slice = Slice::default();
slice
.with_source(source)
.with_location(file_name, line, column);
.with_location(file_path, line, column);
diag.add_slice(slice);
}
}
Expand Down Expand Up @@ -4395,17 +4395,17 @@ fn variadic_fn_diagnostic(
.add_annotation("No code will be generated for this function.", Level::Note);

if let Some(crate::clang::SourceLocation::File {
file_name,
file_path,
line,
column,
..
}) = location.cloned()
{
if let Ok(Some(source)) = get_line(&file_name, line) {
if let Ok(Some(source)) = get_line(&file_path, line) {
let mut slice = Slice::default();
slice
.with_source(source)
.with_location(file_name, line, column);
.with_location(file_path, line, column);
diag.add_slice(slice);
}
}
Expand Down
17 changes: 8 additions & 9 deletions bindgen/diagnostics.rs
Expand Up @@ -2,8 +2,8 @@
//!
//! The entry point of this module is the [`Diagnostic`] type.

use std::fmt::Write;
use std::io::{self, BufRead, BufReader};
use std::path::Path;
use std::{borrow::Cow, fs::File};

use annotate_snippets::{
Expand Down Expand Up @@ -162,25 +162,24 @@ impl<'a> Slice<'a> {
}

/// Set the file, line and column.
pub(crate) fn with_location(
pub(crate) fn with_location<P: AsRef<Path>>(
&mut self,
mut name: String,
path: P,
line: usize,
col: usize,
) -> &mut Self {
write!(name, ":{}:{}", line, col)
.expect("Writing to a string cannot fail");
self.filename = Some(name);
self.filename =
Some(format!("{}:{}:{}", path.as_ref().display(), line, col));
self.line = Some(line);
self
}
}

pub(crate) fn get_line(
filename: &str,
pub(crate) fn get_line<P: AsRef<Path>>(
file_path: P,
line: usize,
) -> io::Result<Option<String>> {
let file = BufReader::new(File::open(filename)?);
let file = BufReader::new(File::open(file_path.as_ref())?);
if let Some(line) = file.lines().nth(line.wrapping_sub(1)) {
return line.map(Some);
}
Expand Down

0 comments on commit 909d926

Please sign in to comment.