Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add custom attributes to fn items #3088

Draft
wants to merge 31 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7440755
feat: add custom attributes to `fn` items
oberrich Jan 11, 2025
027ee8f
chore: run rustfmt
oberrich Jan 11, 2025
9176a56
fix: use canonical name
oberrich Jan 11, 2025
b95ea83
feat: implement for regular functions
oberrich Jan 11, 2025
8bf98e3
chore: run rustfmt
oberrich Jan 11, 2025
3afa60b
fix: store `FunctionKind` in `Function` variant instead of `Attribute…
oberrich Jan 15, 2025
3f811a2
chore: run rustfmt
oberrich Jan 15, 2025
8ce3503
fix: cli tool and some clippy warnings
oberrich Jan 15, 2025
a5add12
feat(bindgen-integration): add test for custom `fn` attributes
oberrich Jan 16, 2025
133ac06
feat(tests): add cli test for custom `fn` attributes
oberrich Jan 16, 2025
a2a5359
fix(tests): remove parse_meta
oberrich Jan 16, 2025
5268ae4
fix(tests): use visitor to traverse syntax tree
oberrich Jan 16, 2025
c2dc13b
fix(integration): add missing newline
oberrich Jan 16, 2025
c7676f0
style(integration): make code more idiomatic
oberrich Jan 16, 2025
eb3f139
fix(integration): add missing `else`
oberrich Jan 16, 2025
9f3c861
feat: add attributes from annotations for `fn` items
oberrich Jan 16, 2025
5de3677
feat(tests): add tests for rustbindgen annotations
oberrich Jan 16, 2025
53a3653
fix: remove IDE inserted types
oberrich Jan 16, 2025
48e7547
chore: run rustfmt
oberrich Jan 16, 2025
94c67b8
attempt adding attributes via annotations
oberrich Jan 22, 2025
0d066fb
attempt adding attributes via annotations
oberrich Jan 22, 2025
5b259ff
add `merge_cfg_attributes` pass and improve overall attribute capabil…
oberrich Jan 23, 2025
c854516
add `merge_cfg_attributes` pass
oberrich Jan 23, 2025
2b9b41b
refactor `merge_cfg_attributes` pass
oberrich Jan 23, 2025
ea82940
added TODO comment
oberrich Jan 23, 2025
9036b10
fix: correctly handle abi and unsafety
oberrich Jan 26, 2025
875348d
fix: only apply attributes relevant to compilation via type aliases
oberrich Jan 26, 2025
98a4881
chore: apply new pattern since MSVR is now >= 1.59.0
oberrich Jan 26, 2025
c769d52
chore: remove completed TODO comments
oberrich Jan 26, 2025
17dbb68
fix: normalize attributes, unified callback dispatch
oberrich Feb 1, 2025
0920eee
fix: msvr issue
oberrich Feb 4, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ clang-sys = "1"
clap = "4"
clap_complete = "4"
env_logger = "0.10.0"
itertools = { version = ">=0.10,<0.14", default-features = false }
itertools = { version = ">=0.10,<0.15", default-features = false, features = ["use_alloc"]}
libloading = "0.8"
log = "0.4"
objc = "0.2"
Expand Down
3 changes: 3 additions & 0 deletions bindgen-integration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ build = "build.rs"
rust-version.workspace = true
edition.workspace = true

[dependencies]
syn = { workspace = true, features = ["full", "visit"] }

[build-dependencies]
bindgen = { workspace = true, default-features = true, features = ["experimental"] }
cc.workspace = true
Expand Down
10 changes: 9 additions & 1 deletion bindgen-integration/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
extern crate bindgen;

use bindgen::callbacks::{
DeriveInfo, IntKind, MacroParsingBehavior, ParseCallbacks,
AttributeItemKind, DeriveInfo, FunctionKind, IntKind, MacroParsingBehavior,
ParseCallbacks,
};
use bindgen::{Builder, EnumVariation, Formatter};
use std::collections::HashSet;
Expand Down Expand Up @@ -140,7 +141,14 @@ impl ParseCallbacks for MacroCallback {
info: &bindgen::callbacks::AttributeInfo<'_>,
) -> Vec<String> {
if info.name == "Test" {
assert_eq!(info.kind, AttributeItemKind::Struct);
vec!["#[cfg_attr(test, derive(PartialOrd))]".into()]
} else if info.name == "coord" {
assert_eq!(
info.kind,
AttributeItemKind::Function(FunctionKind::Function)
);
vec!["#[must_use]".into()]
} else {
vec![]
}
Expand Down
56 changes: 56 additions & 0 deletions bindgen-integration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,62 @@ fn test_custom_derive() {
assert!(!(test1 > test2));
}

#[test]
fn test_custom_fn_attribute() {
use std::env;
use std::fs;
use std::path::Path;
use syn::visit::Visit;
use syn::{parse_file, File, ForeignItem, Item, ItemForeignMod};

let out_dir =
std::env::var("OUT_DIR").expect("OUT_DIR environment variable not set");
let test_file_path = Path::new(&out_dir).join("test.rs");
let file_content = fs::read_to_string(&test_file_path)
.expect("Failed to read test.rs file");
let syntax_tree: File =
parse_file(&file_content).expect("Failed to parse test.rs");

struct FunctionVisitor {
found_coord: bool,
has_must_use: bool,
}

impl<'ast> Visit<'ast> for FunctionVisitor {
fn visit_item_foreign_mod(
&mut self,
foreign_mod: &'ast ItemForeignMod,
) {
for foreign_item in &foreign_mod.items {
if let ForeignItem::Fn(item_fn) = foreign_item {
if item_fn.sig.ident == "coord" {
self.found_coord = true;
self.has_must_use = item_fn
.attrs
.iter()
.any(|attr| attr.path().is_ident("must_use"));
}
}
}
}
}

let mut visitor = FunctionVisitor {
found_coord: false,
has_must_use: false,
};
visitor.visit_file(&syntax_tree);

assert!(
visitor.found_coord,
"The function 'coord' was not found in the source."
);
assert!(
visitor.has_must_use,
"The function 'coord' does not have the #[must_use] attribute."
);
}

#[test]
fn test_custom_attributes() {
// The `add_attributes` callback should have added `#[cfg_attr(test, derive(PartialOrd))])`
Expand Down

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

6 changes: 6 additions & 0 deletions bindgen-tests/tests/expectations/tests/attribute-custom.rs

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

3 changes: 2 additions & 1 deletion bindgen-tests/tests/headers/attribute-custom-cli.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --default-enum-style rust --default-non-copy-union-style manually_drop --no-default=".*" --no-hash=".*" --no-partialeq=".*" --no-debug=".*" --no-copy=".*" --with-attribute-custom="foo_[^e].*=#[doc(hidden)]" --with-attribute-custom-struct="foo.*=#[derive(Default)]" --with-attribute-custom-enum="foo.*=#[cfg_attr(test, derive(PartialOrd, Copy))]" --with-attribute-custom-union="foo.*=#[derive(Clone)],#[derive(Copy)]"
// bindgen-flags: --default-enum-style rust --default-non-copy-union-style manually_drop --no-default=".*" --no-hash=".*" --no-partialeq=".*" --no-debug=".*" --no-copy=".*" --with-attribute-custom="foo_[^e].*=#[doc(hidden)]" --with-attribute-custom-struct="foo.*=#[derive(Default)]" --with-attribute-custom-enum="foo.*=#[cfg_attr(test, derive(PartialOrd, Copy))]" --with-attribute-custom-union="foo.*=#[derive(Clone)],#[derive(Copy)]" --with-attribute-custom-function="foo.*=#[must_use],#[doc(hidden)]"
struct foo_struct {
int inner;
};
Expand All @@ -12,3 +12,4 @@ union foo_union {
struct non_matching {
int inner;
};
int foo_function() { return 1; }
5 changes: 5 additions & 0 deletions bindgen-tests/tests/headers/attribute-custom.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,8 @@ struct my_type2 {
struct my_type3 {
unsigned long a;
};

/**
* <div rustbindgen attribute="#[must_use]" attribute="#[doc(hidden)]"></div>
*/
int function_attributes() { return 1; }
30 changes: 23 additions & 7 deletions bindgen/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
pub use crate::ir::analysis::DeriveTrait;
pub use crate::ir::derive::CanDerive as ImplementsTrait;
pub use crate::ir::enum_ty::{EnumVariantCustomBehavior, EnumVariantValue};
pub use crate::ir::function::FunctionKind;
pub use crate::ir::int::IntKind;
use crate::HashSet;
use std::fmt;

/// An enum to allow ignoring parsing of macros.
Expand Down Expand Up @@ -133,9 +135,7 @@ pub trait ParseCallbacks: fmt::Debug {
///
/// If no additional attributes are wanted, this function should return an
/// empty `Vec`.
fn add_attributes(&self, _info: &AttributeInfo<'_>) -> Vec<String> {
vec![]
}
fn process_attributes(&self, _info: &AttributeInfo<'_>, _attributes: &mut HashSet<String>) {}

/// Process a source code comment.
fn process_comment(&self, _comment: &str) -> Option<String> {
Expand Down Expand Up @@ -231,15 +231,31 @@ pub struct DeriveInfo<'a> {
pub kind: TypeKind,
}

/// Relevant information about a type to which new attributes will be added using
/// Relevant information about an item to which new attributes will be added using
/// [`ParseCallbacks::add_attributes`].
#[derive(Debug)]
#[non_exhaustive]
pub struct AttributeInfo<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add the doc comment to this struct?
In my case, I would like to feature guard items, based on information provided in doc comments.
In my fork of bindgen I added a custom callback for this purpose, but perhaps I could use this callback in the future.

Copy link
Author

@oberrich oberrich Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can already do this on alias branch:

if attrs.iter().any(|attr| attr.contains("doc")) {
   attrs.insert(r#"#[cfg(feature="guard")]"#.to_owned());
   println!("cargo:warning={attrs:#?}")
}

yields

[phnt 0.1.2] cargo:warning={
[phnt 0.1.2]     "# [repr (C)]",
[phnt 0.1.2]     "# [derive (Debug , Copy , Clone)]",
[phnt 0.1.2]     "# [doc = \"The NTPSS_MEMORY_BULK_INFORMATION structure is used to query basic memory information in bulk for a process.\"]",
[phnt 0.1.2]     "#[cfg(feature=\"guard\")]",
[phnt 0.1.2] }

A few issues I've come up with:

  • Formatting of attributes is inconsistent. We need to consistently format attributes. I've made a function that normalizes attributes.
  • Might be worth to split attributes into <attr, args> perhaps for easier filtering. This requires extra parsing iirc.

/// The name of the type.
/// The name of the item.
pub name: &'a str,
/// The kind of the type.
pub kind: TypeKind,
/// The kind of the item.
pub kind: AttributeItemKind,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
/// The kind of the current item.
pub enum AttributeItemKind {
/// The item is a Rust `struct`.
Struct,
/// The item is a Rust `enum`.
Enum,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the callback only called on the main enum / struct / union definition, or also for each of the struct fields / enum variants? For example, one might want to feature guard certain struct fields.

Copy link
Author

@oberrich oberrich Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only called on the actual enums I think; they can be implemented in different ways so I think it might be a bit harder to do. We should definitely add more info/context to all AttributeItemKind variants, perhaps we can just wrap a list of struct fields/enum variants into the variants.

Lots of variants in this sentence.

/// The item is a Rust `union`.
Union,
/// The item is a Rust variable.
Var,
/// The item is a Rust `fn`.
Function(FunctionKind),
// TODO: Add `Alias` variant and more information about other items
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down
Loading