Skip to content

Commit

Permalink
Handle all cases in cmp_by_source_order.
Browse files Browse the repository at this point in the history
  • Loading branch information
@reitermarkus
reitermarkus committed Jun 17, 2023
1 parent d5e8204 commit 83f43a0
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 27 deletions.
163 changes: 138 additions & 25 deletions bindgen/clang.rs
View file
Expand Up @@ -4,7 +4,7 @@
#![allow(non_upper_case_globals, dead_code)]
#![deny(clippy::missing_docs_in_private_items)]

use crate::ir::context::BindgenContext;
use crate::ir::context::{BindgenContext, IncludeLocation};
use clang_sys::*;
use std::cmp;

Expand Down Expand Up @@ -552,32 +552,145 @@ impl Cursor {
return offset.cmp(&other_offset);
}

// `None` here means `file`/`other_file` is the main header file.
let include_location = ctx.included_file_location(&file);
let other_include_location = ctx.included_file_location(&other_file);
let mut include_location = ctx.included_file_location(&file);
let mut other_include_location =
ctx.included_file_location(&other_file);

match (include_location, other_include_location) {
// The main header file (`None`) comes after header passed as CLI argument (`Some((None, _))`).
(None, Some((None, _))) => cmp::Ordering::Greater,
(Some((None, _)), None) => cmp::Ordering::Less,
// 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.
(Some((Some(file2), offset2)), _) if file2 == other_file => {
offset2.cmp(&other_offset)
}
(_, Some((Some(other_file2), other_offset2)))
if file == other_file2 =>
{
offset.cmp(&other_offset2)
}
// If both items were included in the same file, compare the offset of their `#include` directives.
(Some((file2, offset2)), Some((other_file2, other_offset2)))
if file2 == other_file2 =>
{
offset2.cmp(&other_offset2)
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),

Check warning on line 576 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
// 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);
}

// Continue checking one level up.
include_location = ctx.included_file_location(&file2);

Check warning on line 591 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}
(
Main,
File {
file_name: ref other_file2,
offset: other_offset2,
},
) => {
if file == *other_file2 {
return offset.cmp(&other_offset2);

Check warning on line 601 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}

// Continue checking one level up.
other_include_location =
ctx.included_file_location(&other_file2);

Check warning on line 606 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}
(
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);

Check warning on line 620 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}

// 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 mut include_location =

Check warning on line 627 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

variable does not need to be mutable
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)

Check warning on line 648 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
{
return offset2.cmp(&other_offset2);

Check warning on line 650 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}

if let Some(other_offset2) =
offset_in_ancestor(other_file2.clone(), &file2)

Check warning on line 654 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
{
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);

Check warning on line 680 in bindgen/clang.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

this expression creates a reference which is immediately dereferenced by the compiler
}
(
Cli { .. },
File {
file_name: other_file2,
..
},
) => {
// Continue checking one level up.
other_include_location =
ctx.included_file_location(&other_file2);
}
}
// Otherwise, keep the original sorting.
_ => cmp::Ordering::Equal,
}
}

Expand Down
21 changes: 19 additions & 2 deletions bindgen/ir/context.rs
View file
Expand Up @@ -305,6 +305,14 @@ enum TypeKey {
Declaration(Cursor),
}

/// Specifies where a header was included from.
#[derive(Debug)]
pub(crate) enum IncludeLocation {
Main,

Check failure on line 311 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a variant
Cli { offset: usize },

Check failure on line 312 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a variant

Check failure on line 312 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a struct field
File { file_name: String, offset: usize },

Check failure on line 313 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a variant

Check failure on line 313 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a struct field

Check failure on line 313 in bindgen/ir/context.rs

View workflow job for this annotation

GitHub Actions / rustfmt-clippy 

missing documentation for a struct field
}

/// A context used during parsing and generation of structs.
#[derive(Debug)]
pub(crate) struct BindgenContext {
Expand Down Expand Up @@ -659,8 +667,17 @@ If you encounter an error missing from this list, please file an issue or a PR!"
pub(crate) fn included_file_location(
&self,
included_file: &str,
) -> Option<(Option<String>, usize)> {
self.includes.get(included_file).cloned()
) -> IncludeLocation {
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 }
}
}
}

/// Add an included file.
Expand Down

0 comments on commit 83f43a0

Please sign in to comment.