Skip to content

implement clone on builder #2134

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ cexpr = "0.6"
# This kinda sucks: https://github.com/rust-lang/cargo/issues/1982
clap = { version = "2", optional = true }
clang-sys = { version = "1", features = ["clang_6_0"] }
dyn-clone = { version = "1", optional = true }
lazycell = "1"
lazy_static = "1"
peeking_take_while = "0.1.2"
Expand All @@ -71,6 +72,8 @@ version = "0.4"

[features]
default = ["logging", "clap", "runtime", "which-rustfmt"]
# Implement the `Clone` trait on `bindgen::Builder`
builder-clone = ["dyn-clone"]
logging = ["env_logger", "log"]
static = ["clang-sys/static"]
runtime = ["clang-sys/runtime"]
Expand Down
90 changes: 90 additions & 0 deletions src/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl Default for MacroParsingBehavior {

/// A trait to allow configuring different kinds of types in different
/// situations.
#[cfg(not(feature = "builder-clone"))]
pub trait ParseCallbacks: fmt::Debug + UnwindSafe {
/// This function will be run on every macro that is identified.
fn will_parse_macro(&self, _name: &str) -> MacroParsingBehavior {
Expand Down Expand Up @@ -104,3 +105,92 @@ pub trait ParseCallbacks: fmt::Debug + UnwindSafe {
vec![]
}
}

/// The same trait as above, but clonable so that it can be included as a
/// field in Builder when the builder-clone feature is enabled such that
/// Builder implements Clone.
#[cfg(feature = "builder-clone")]
pub trait ParseCallbacks:
Copy link
Contributor

Choose a reason for hiding this comment

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

It is really unfortunate to duplicate all the trait here. Can we avoid this by using a macro instead?

fmt::Debug + UnwindSafe + dyn_clone::DynClone
{
/// This function will be run on every macro that is identified.
fn will_parse_macro(&self, _name: &str) -> MacroParsingBehavior {
MacroParsingBehavior::Default
}

/// The integer kind an integer macro should have, given a name and the
/// value of that macro, or `None` if you want the default to be chosen.
fn int_macro(&self, _name: &str, _value: i64) -> Option<IntKind> {
None
}

/// This will be run on every string macro. The callback cannot influence the further
/// treatment of the macro, but may use the value to generate additional code or configuration.
fn str_macro(&self, _name: &str, _value: &[u8]) {}

/// This will be run on every function-like macro. The callback cannot
/// influence the further treatment of the macro, but may use the value to
/// generate additional code or configuration.
///
/// The first parameter represents the name and argument list (including the
/// parentheses) of the function-like macro. The second parameter represents
/// the expansion of the macro as a sequence of tokens.
fn func_macro(&self, _name: &str, _value: &[&[u8]]) {}

/// This function should return whether, given an enum variant
/// name, and value, this enum variant will forcibly be a constant.
fn enum_variant_behavior(
&self,
_enum_name: Option<&str>,
_original_variant_name: &str,
_variant_value: EnumVariantValue,
) -> Option<EnumVariantCustomBehavior> {
None
}

/// Allows to rename an enum variant, replacing `_original_variant_name`.
fn enum_variant_name(
&self,
_enum_name: Option<&str>,
_original_variant_name: &str,
_variant_value: EnumVariantValue,
) -> Option<String> {
None
}

/// Allows to rename an item, replacing `_original_item_name`.
fn item_name(&self, _original_item_name: &str) -> Option<String> {
None
}

/// This will be called on every file inclusion, with the full path of the included file.
fn include_file(&self, _filename: &str) {}

/// This will be called to determine whether a particular blocklisted type
/// implements a trait or not. This will be used to implement traits on
/// other types containing the blocklisted type.
///
/// * `None`: use the default behavior
/// * `Some(ImplementsTrait::Yes)`: `_name` implements `_derive_trait`
/// * `Some(ImplementsTrait::Manually)`: any type including `_name` can't
/// derive `_derive_trait` but can implemented it manually
/// * `Some(ImplementsTrait::No)`: `_name` doesn't implement `_derive_trait`
fn blocklisted_type_implements_trait(
&self,
_name: &str,
_derive_trait: DeriveTrait,
) -> Option<ImplementsTrait> {
None
}

/// Provide a list of custom derive attributes.
///
/// If no additional attributes are wanted, this function should return an
/// empty `Vec`.
fn add_derives(&self, _name: &str) -> Vec<String> {
vec![]
}
}

#[cfg(feature = "builder-clone")]
dyn_clone::clone_trait_object!(ParseCallbacks);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate on why this is needed instead of just having ParseCallbacks: Clone?

1 change: 1 addition & 0 deletions src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,7 @@ impl Drop for Diagnostic {
}

/// A file which has not been saved to disk.
#[derive(Clone)]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not safe. UnsavedFile keeps pointers to its inner CStrings so at the very least the clone implementation needs to be more subtle.

pub struct UnsavedFile {
x: CXUnsavedFile,
/// The name of the unsaved file. Kept here to avoid leaving dangling pointers in
Expand Down
2 changes: 1 addition & 1 deletion src/deps.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// Generating build depfiles from parsed bindings.
use std::{collections::BTreeSet, path::PathBuf};

#[derive(Debug)]
#[derive(Clone, Debug)]
pub(crate) struct DepfileSpec {
pub output_module: String,
pub depfile_path: PathBuf,
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ impl Default for CodegenConfig {
/// End-users of the crate may need to set the `BINDGEN_EXTRA_CLANG_ARGS` environment variable to
/// add additional arguments. For example, to build against a different sysroot a user could set
/// `BINDGEN_EXTRA_CLANG_ARGS` to `--sysroot=/path/to/sysroot`.
#[cfg_attr(feature = "builder-clone", derive(Clone))]
#[derive(Debug, Default)]
pub struct Builder {
options: BindgenOptions,
Expand Down Expand Up @@ -1663,6 +1664,7 @@ impl Builder {
}

/// Configuration options for generated bindings.
#[cfg_attr(feature = "builder-clone", derive(Clone))]
#[derive(Debug)]
struct BindgenOptions {
/// The set of types that have been blocklisted and should not appear
Expand Down Expand Up @@ -2656,7 +2658,7 @@ fn get_target_dependent_env_var(var: &str) -> Option<String> {
/// .parse_callbacks(Box::new(bindgen::CargoCallbacks))
/// .generate();
/// ```
#[derive(Debug)]
#[derive(Clone, Debug)]
pub struct CargoCallbacks;

impl callbacks::ParseCallbacks for CargoCallbacks {
Expand Down
2 changes: 1 addition & 1 deletion src/regex_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use regex::RegexSet as RxSet;
use std::cell::Cell;

/// A dynamic set of regular expressions.
#[derive(Debug, Default)]
#[derive(Clone, Debug, Default)]
pub struct RegexSet {
items: Vec<String>,
/// Whether any of the items in the set was ever matched. The length of this
Expand Down
4 changes: 2 additions & 2 deletions tests/parse_callbacks/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bindgen::callbacks::*;

#[derive(Debug)]
#[derive(Clone, Debug)]
struct EnumVariantRename;

impl ParseCallbacks for EnumVariantRename {
Expand All @@ -14,7 +14,7 @@ impl ParseCallbacks for EnumVariantRename {
}
}

#[derive(Debug)]
#[derive(Clone, Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

struct BlocklistedTypeImplementsTrait;

impl ParseCallbacks for BlocklistedTypeImplementsTrait {
Expand Down