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

Feature 699 constified enum module #741

Merged
Merged
74 changes: 67 additions & 7 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ use std::mem;
use std::ops;
use syntax::abi;
use syntax::ast;
use syntax::codemap::{Span, respan};
use syntax::codemap::{DUMMY_SP, Span, respan};
use syntax::ptr::P;

// Name of type defined in constified enum module
pub static CONSTIFIED_ENUM_MODULE_REPR_NAME: &'static str = "Type";

fn root_import_depth(ctx: &BindgenContext, item: &Item) -> usize {
if !ctx.options().enable_cxx_namespaces {
return 0;
Expand Down Expand Up @@ -244,7 +247,6 @@ impl ForeignModBuilder {
}

fn build(self, ctx: &BindgenContext) -> P<ast::Item> {
use syntax::codemap::DUMMY_SP;
P(ast::Item {
ident: ctx.rust_ident(""),
id: ast::DUMMY_NODE_ID,
Expand Down Expand Up @@ -2001,6 +2003,10 @@ enum EnumBuilder<'a> {
aster: P<ast::Item>,
},
Consts { aster: P<ast::Item> },
ModuleConsts {
module_name: &'a str,
module_items: Vec<P<ast::Item>>,
},
}

impl<'a> EnumBuilder<'a> {
Expand All @@ -2010,7 +2016,8 @@ impl<'a> EnumBuilder<'a> {
name: &'a str,
repr: P<ast::Ty>,
bitfield_like: bool,
constify: bool)
constify: bool,
constify_module: bool)
-> Self {
if bitfield_like {
EnumBuilder::Bitfield {
Expand All @@ -2022,8 +2029,20 @@ impl<'a> EnumBuilder<'a> {
.build(),
}
} else if constify {
EnumBuilder::Consts {
aster: aster.type_(name).build_ty(repr),
if constify_module {
let type_definition = aster::item::ItemBuilder::new()
.pub_()
.type_(CONSTIFIED_ENUM_MODULE_REPR_NAME)
.build_ty(repr);

EnumBuilder::ModuleConsts {
module_name: name,
module_items: vec![type_definition],
}
} else {
EnumBuilder::Consts {
aster: aster.type_(name).build_ty(repr),
}
}
} else {
EnumBuilder::Rust(aster.enum_(name))
Expand Down Expand Up @@ -2095,6 +2114,27 @@ impl<'a> EnumBuilder<'a> {
result.push(constant);
self
}
EnumBuilder::ModuleConsts { module_name, module_items, .. } => {
// Variant type
let inside_module_type =
aster::AstBuilder::new().ty().id(CONSTIFIED_ENUM_MODULE_REPR_NAME);

let constant = aster::AstBuilder::new()
.item()
.pub_()
.const_(&*variant_name)
.expr()
.build(expr)
.build(inside_module_type.clone());

let mut module_items = module_items.clone();
module_items.push(constant);

EnumBuilder::ModuleConsts {
module_name,
module_items,
}
}
}
}

Expand Down Expand Up @@ -2160,6 +2200,22 @@ impl<'a> EnumBuilder<'a> {
aster
}
EnumBuilder::Consts { aster, .. } => aster,
EnumBuilder::ModuleConsts { module_items, module_name, .. } => {
// Create module item with type and variant definitions
let module_item = P(ast::Item {
ident: ast::Ident::from_str(module_name),
attrs: vec![],
id: ast::DUMMY_NODE_ID,
node: ast::ItemKind::Mod(ast::Mod {
inner: DUMMY_SP,
items: module_items,
}),
vis: ast::Visibility::Public,
span: DUMMY_SP,
});

module_item
}
}
}
}
Expand Down Expand Up @@ -2225,7 +2281,10 @@ impl CodeGenerator for Enum {
.any(|v| ctx.options().bitfield_enums.matches(&v.name())))
};

let is_constified_enum = {
let is_constified_enum_module = self.is_constified_enum_module(ctx, item);

let is_constified_enum = {
is_constified_enum_module ||
ctx.options().constified_enums.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants()
Expand Down Expand Up @@ -2300,7 +2359,8 @@ impl CodeGenerator for Enum {
&name,
repr,
is_bitfield,
is_constified_enum);
is_constified_enum,
is_constified_enum_module);

// A map where we keep a value -> variant relation.
let mut seen_values = HashMap::<_, String>::new();
Expand Down
15 changes: 15 additions & 0 deletions src/ir/enum_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ use super::item::Item;
use super::ty::TypeKind;
use clang;
use ir::annotations::Annotations;
use ir::item::ItemCanonicalName;
use parse::{ClangItemParser, ParseError};

/// An enum representing custom handling that can be given to a variant.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum EnumVariantCustomBehavior {
/// This variant will be a module containing constants.
ModuleConstify,
/// This variant will be constified, that is, forced to generate a constant.
Constify,
/// This variant will be hidden entirely from the resulting enum.
Expand Down Expand Up @@ -121,6 +124,18 @@ impl Enum {
});
Ok(Enum::new(repr, variants))
}

/// Whether the enum should be an constified enum module
pub fn is_constified_enum_module(&self, ctx: &BindgenContext, item: &Item) -> bool {
let name = item.canonical_name(ctx);
let enum_ty = item.expect_type();

ctx.options().constified_enum_modules.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants()
.iter()
.any(|v| ctx.options().constified_enum_modules.matches(&v.name())))
}
}

/// A single enum variant, to be contained only in an enum.
Expand Down
21 changes: 20 additions & 1 deletion src/ir/item.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Bindgen's core intermediate representation type.

use super::super::codegen::CONSTIFIED_ENUM_MODULE_REPR_NAME;
use super::annotations::Annotations;
use super::context::{BindgenContext, ItemId, PartialType};
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
Expand Down Expand Up @@ -1443,10 +1444,28 @@ impl ItemCanonicalPath for Item {
fn namespace_aware_canonical_path(&self,
ctx: &BindgenContext)
-> Vec<String> {
let path = self.canonical_path(ctx);
let mut path = self.canonical_path(ctx);
let mut is_constified_module_enum = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a helper function that returns this instead.

if let ItemKind::Type(ref type_) = self.kind {
if let Some(ref type_) = type_.safe_canonical_type(ctx) {
if let TypeKind::Enum(ref enum_) = *type_.kind() {
if enum_.is_constified_enum_module(ctx, self) {
// Type alias is inside a module
is_constified_module_enum = true;
}
}
}
}
if ctx.options().enable_cxx_namespaces {
if is_constified_module_enum {
path.push(CONSTIFIED_ENUM_MODULE_REPR_NAME.into());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be in canonical_path instead, and we'd just keep this as it was. This will make this function way less error-prone, see below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the check should be in namespace_aware_canonical_path. If canonical_path appends ::Type to the path, then namespace_aware_canonical_path will have to also check if the item is a constified module enum. Both functions would need to be changed instead of just namespace_aware_canonical_path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I guess, but that relies on canonical_path users to not care about this... Which I guess holds, for now. Fine then.

}
return path;
}
if is_constified_module_enum {
Copy link
Contributor

@emilio emilio Jun 17, 2017

Choose a reason for hiding this comment

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

To fix the last test-case I commented about, you should remove this early return and let it arrive to the last return.

For that to work, you need to add the REPR_NAME to the path, so this method could be structured like:

if is_constified_module_enum {
    path.push(CONSTIFIED_ENUM_MODULE_REPR_NAME.into());
}

if ctx.options().enable_cxx_namespaces {
    return path;
}

if ctx.options().disable_name_namespacing {
    let trailing_segments = if is_constified_module_enum { 2 } else { 1 };
    return path[..path.len() - trailing_segments].iter().cloned().collect();
}

return vec![path[1..].join("_")];

return vec![path.last().unwrap().clone(),
CONSTIFIED_ENUM_MODULE_REPR_NAME.into()];
}
if ctx.options().disable_name_namespacing {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this branch is correct.

return vec![path.last().unwrap().clone()];
}
Expand Down
29 changes: 27 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,16 @@ impl Builder {
})
.count();

self.options
.constified_enum_modules
.get_items()
.iter()
.map(|item| {
output_vector.push("--constified-enum-module".into());
output_vector.push(item.trim_left_matches("^").trim_right_matches("$").into());
})
.count();

self.options
.hidden_types
.get_items()
Expand Down Expand Up @@ -562,8 +572,8 @@ impl Builder {
self
}

/// Mark the given enum (or set of enums, if using a pattern) as being
/// constant.
/// Mark the given enum (or set of enums, if using a pattern) as a set of
/// constants.
///
/// This makes bindgen generate constants instead of enums. Regular
/// expressions are supported.
Expand All @@ -572,6 +582,16 @@ impl Builder {
self
}

/// Mark the given enum (or set of enums, if using a pattern) as a set of
/// constants that should be put into a module.
///
/// This makes bindgen generate a modules containing constants instead of
/// enums. Regular expressions are supported.
pub fn constified_enum_module<T: AsRef<str>>(mut self, arg: T) -> Builder {
self.options.constified_enum_modules.insert(arg);
self
}

/// Add a string to prepend to the generated bindings. The string is passed
/// through without any modification.
pub fn raw_line<T: Into<String>>(mut self, arg: T) -> Builder {
Expand Down Expand Up @@ -813,6 +833,9 @@ pub struct BindgenOptions {
/// The enum patterns to mark an enum as constant.
pub constified_enums: RegexSet,

/// The enum patterns to mark an enum as a module of constants.
pub constified_enum_modules: RegexSet,

/// Whether we should generate builtins or not.
pub builtins: bool,

Expand Down Expand Up @@ -935,6 +958,7 @@ impl BindgenOptions {
self.hidden_types.build();
self.opaque_types.build();
self.bitfield_enums.build();
self.constified_enum_modules.build();
self.constified_enums.build();
}
}
Expand All @@ -949,6 +973,7 @@ impl Default for BindgenOptions {
whitelisted_vars: Default::default(),
bitfield_enums: Default::default(),
constified_enums: Default::default(),
constified_enum_modules: Default::default(),
builtins: false,
links: vec![],
emit_ast: false,
Expand Down
18 changes: 16 additions & 2 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ pub fn builder_from_flags<I>
.takes_value(true)
.multiple(true)
.number_of_values(1),
Arg::with_name("constified-enum-module")
.long("constified-enum-module")
.help("Mark any enum whose name matches <regex> as a module of \
constants instead of an enumeration. This option \
implies \"--constified-enum.\"")
.value_name("regex")
.takes_value(true)
.multiple(true)
.number_of_values(1),
Arg::with_name("blacklist-type")
.long("blacklist-type")
.help("Mark <type> as hidden.")
Expand Down Expand Up @@ -222,12 +231,17 @@ pub fn builder_from_flags<I>
}
}

if let Some(bitfields) = matches.values_of("constified-enum") {
for regex in bitfields {
if let Some(constifieds) = matches.values_of("constified-enum") {
for regex in constifieds {
builder = builder.constified_enum(regex);
}
}

if let Some(constified_mods) = matches.values_of("constified-enum-module") {
for regex in constified_mods {
builder = builder.constified_enum_module(regex);
}
}
if let Some(hidden_types) = matches.values_of("blacklist-type") {
for ty in hidden_types {
builder = builder.hide_type(ty);
Expand Down
45 changes: 45 additions & 0 deletions tests/expectations/tests/constify-module-enums-basic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]


pub mod foo {
pub type Type = ::std::os::raw::c_uint;
pub const THIS: Type = 0;
pub const SHOULD_BE: Type = 1;
pub const A_CONSTANT: Type = 2;
}
pub use self::foo::Type as foo_alias1;
pub use self::foo_alias1 as foo_alias2;
#[repr(C)]
#[derive(Debug, Copy)]
pub struct bar {
pub this_should_work: foo::Type,
}
#[test]
fn bindgen_test_layout_bar() {
assert_eq!(::std::mem::size_of::<bar>() , 4usize , concat ! (
"Size of: " , stringify ! ( bar ) ));
assert_eq! (::std::mem::align_of::<bar>() , 4usize , concat ! (
"Alignment of " , stringify ! ( bar ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const bar ) ) . this_should_work as * const _
as usize } , 0usize , concat ! (
"Alignment of field: " , stringify ! ( bar ) , "::" ,
stringify ! ( this_should_work ) ));
}
impl Clone for bar {
fn clone(&self) -> Self { *self }
}
impl Default for bar {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
extern "C" {
pub fn func1(arg1: foo::Type, arg2: *mut foo::Type,
arg3: *mut *mut foo::Type) -> *mut foo::Type;
}
extern "C" {
pub fn func2(arg1: foo_alias1, arg2: *mut foo_alias1,
arg3: *mut *mut foo_alias1) -> *mut foo_alias1;
}
Loading