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.

Sign up for GitHub

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

Jump to bottom

Generate fields as non-pub if they would be access restricted in C++. #1968

Merged
merged 1 commit into from Jan 29, 2021
Merged

Generate fields as non-pub if they would be access restricted in C++. #1968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/clang.rs
View file
Open in desktop
Expand Up @@ -632,6 +632,15 @@ impl Cursor {
unsafe { clang_getCXXAccessSpecifier(self.x) }
}

/// Is the cursor's referrent publically accessible in C++?
///
/// Returns true if self.access_specifier() is `CX_CXXPublic` or
/// `CX_CXXInvalidAccessSpecifier`.
pub fn public_accessible(&self) -> bool {
let access = self.access_specifier();
access == CX_CXXPublic || access == CX_CXXInvalidAccessSpecifier
}

/// Is this cursor's referent a field declaration that is marked as
/// `mutable`?
pub fn is_mutable_field(&self) -> bool {
Expand Down
54 changes: 37 additions & 17 deletions src/codegen/mod.rs
View file
Open in desktop
Expand Up @@ -1271,10 +1271,11 @@ impl<'a> FieldCodegen<'a> for FieldData {
}
}

let is_private = self
.annotations()
.private_fields()
.unwrap_or(fields_should_be_private);
let is_private = (!self.is_public() &&
ctx.options().respect_cxx_access_specs) ||
self.annotations()
.private_fields()
.unwrap_or(fields_should_be_private);

let accessor_kind =
self.annotations().accessor_kind().unwrap_or(accessor_kind);
Expand Down Expand Up @@ -1395,6 +1396,17 @@ impl Bitfield {
}
}

fn access_specifier(
ctx: &BindgenContext,
is_pub: bool,
) -> proc_macro2::TokenStream {
if is_pub || !ctx.options().respect_cxx_access_specs {
quote! { pub }
} else {
quote! {}
}
}

impl<'a> FieldCodegen<'a> for BitfieldUnit {
type Extra = ();

Expand Down Expand Up @@ -1455,11 +1467,6 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
let unit_field_name = format!("_bitfield_{}", self.nth());
let unit_field_ident = ctx.rust_ident(&unit_field_name);

let field = quote! {
pub #unit_field_ident : #field_ty ,
};
fields.extend(Some(field));

let ctor_name = self.ctor_name();
let mut ctor_params = vec![];
let mut ctor_impl = quote! {};
Expand All @@ -1468,6 +1475,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
// implement AsRef<[u8]> / AsMut<[u8]> / etc.
let mut generate_ctor = layout.size <= RUST_DERIVE_IN_ARRAY_LIMIT;

let mut access_spec = !fields_should_be_private;
for bf in self.bitfields() {
// Codegen not allowed for anonymous bitfields
if bf.name().is_none() {
Expand All @@ -1478,6 +1486,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
continue;
}

access_spec &= bf.is_public();
let mut bitfield_representable_as_int = true;

bf.codegen(
Expand Down Expand Up @@ -1511,10 +1520,17 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
ctor_impl = bf.extend_ctor_impl(ctx, param_name, ctor_impl);
}

let access_spec = access_specifier(ctx, access_spec);

let field = quote! {
#access_spec #unit_field_ident : #field_ty ,
};
fields.extend(Some(field));

if generate_ctor {
methods.extend(Some(quote! {
#[inline]
pub fn #ctor_name ( #( #ctor_params ),* ) -> #unit_field_ty {
#access_spec fn #ctor_name ( #( #ctor_params ),* ) -> #unit_field_ty {
let mut __bindgen_bitfield_unit: #unit_field_ty = Default::default();
#ctor_impl
__bindgen_bitfield_unit
Expand Down Expand Up @@ -1550,7 +1566,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
fn codegen<F, M>(
&self,
ctx: &BindgenContext,
_fields_should_be_private: bool,
fields_should_be_private: bool,
_codegen_depth: usize,
_accessor_kind: FieldAccessorKind,
parent: &CompInfo,
Expand Down Expand Up @@ -1590,13 +1606,16 @@ impl<'a> FieldCodegen<'a> for Bitfield {
bitfield_ty.to_rust_ty_or_opaque(ctx, bitfield_ty_item);

let offset = self.offset_into_unit();

let width = self.width() as u8;
let access_spec = access_specifier(
ctx,
self.is_public() && !fields_should_be_private,
);

if parent.is_union() && !parent.can_be_rust_union(ctx) {
methods.extend(Some(quote! {
#[inline]
pub fn #getter_name(&self) -> #bitfield_ty {
#access_spec fn #getter_name(&self) -> #bitfield_ty {
unsafe {
::#prefix::mem::transmute(
self.#unit_field_ident.as_ref().get(#offset, #width)
Expand All @@ -1606,7 +1625,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
}

#[inline]
pub fn #setter_name(&mut self, val: #bitfield_ty) {
#access_spec fn #setter_name(&mut self, val: #bitfield_ty) {
unsafe {
let val: #bitfield_int_ty = ::#prefix::mem::transmute(val);
self.#unit_field_ident.as_mut().set(
Expand All @@ -1620,7 +1639,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
} else {
methods.extend(Some(quote! {
#[inline]
pub fn #getter_name(&self) -> #bitfield_ty {
#access_spec fn #getter_name(&self) -> #bitfield_ty {
unsafe {
::#prefix::mem::transmute(
self.#unit_field_ident.get(#offset, #width)
Expand All @@ -1630,7 +1649,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
}

#[inline]
pub fn #setter_name(&mut self, val: #bitfield_ty) {
#access_spec fn #setter_name(&mut self, val: #bitfield_ty) {
unsafe {
let val: #bitfield_int_ty = ::#prefix::mem::transmute(val);
self.#unit_field_ident.set(
Expand Down Expand Up @@ -1717,8 +1736,9 @@ impl CodeGenerator for CompInfo {

struct_layout.saw_base(inner_item.expect_type());

let access_spec = access_specifier(ctx, base.is_public());
fields.push(quote! {
pub #field_name: #inner,
#access_spec #field_name: #inner,
});
}
}
Expand Down
52 changes: 44 additions & 8 deletions src/ir/comp.rs
View file
Open in desktop
Expand Up @@ -148,6 +148,9 @@ pub trait FieldMethods {
/// If this is a bitfield, how many bits does it need?
fn bitfield_width(&self) -> Option<u32>;

/// Is this feild declared public?
fn is_public(&self) -> bool;

/// Get the annotations for this field.
fn annotations(&self) -> &Annotations;

Expand Down Expand Up @@ -412,6 +415,10 @@ impl FieldMethods for Bitfield {
self.data.bitfield_width()
}

fn is_public(&self) -> bool {
self.data.is_public()
}

fn annotations(&self) -> &Annotations {
self.data.annotations()
}
Expand All @@ -436,6 +443,7 @@ impl RawField {
comment: Option<String>,
annotations: Option<Annotations>,
bitfield_width: Option<u32>,
public: bool,
offset: Option<usize>,
) -> RawField {
RawField(FieldData {
Expand All @@ -444,6 +452,7 @@ impl RawField {
comment,
annotations: annotations.unwrap_or_default(),
bitfield_width,
public,
offset,
})
}
Expand All @@ -466,6 +475,10 @@ impl FieldMethods for RawField {
self.0.bitfield_width()
}

fn is_public(&self) -> bool {
self.0.is_public()
}

fn annotations(&self) -> &Annotations {
self.0.annotations()
}
Expand Down Expand Up @@ -878,6 +891,9 @@ pub struct FieldData {
/// If this field is a bitfield, and how many bits does it contain if it is.
bitfield_width: Option<u32>,

/// If the C++ field is declared `public`
public: bool,

/// The offset of the field (in bits)
offset: Option<usize>,
}
Expand All @@ -899,6 +915,10 @@ impl FieldMethods for FieldData {
self.bitfield_width
}

fn is_public(&self) -> bool {
self.public
}

fn annotations(&self) -> &Annotations {
&self.annotations
}
Expand Down Expand Up @@ -934,6 +954,8 @@ pub struct Base {
pub kind: BaseKind,
/// Name of the field in which this base should be stored.
pub field_name: String,
/// Whether this base is inherited from publically.
pub is_pub: bool,
}

impl Base {
Expand Down Expand Up @@ -961,6 +983,11 @@ impl Base {

true
}

/// Whether this base is inherited from publically.
pub fn is_public(&self) -> bool {
self.is_pub
}
}

/// A compound type.
Expand Down Expand Up @@ -1230,7 +1257,7 @@ impl CompInfo {
let mut maybe_anonymous_struct_field = None;
cursor.visit(|cur| {
if cur.kind() != CXCursor_FieldDecl {
if let Some((ty, clang_ty, offset)) =
if let Some((ty, clang_ty, public, offset)) =
maybe_anonymous_struct_field.take()
{
if cur.kind() == CXCursor_TypedefDecl &&
Expand All @@ -1242,16 +1269,17 @@ impl CompInfo {
// anonymous field. Detect that case here, and do
// nothing.
} else {
let field =
RawField::new(None, ty, None, None, None, offset);
let field = RawField::new(
None, ty, None, None, None, public, offset,
);
ci.fields.append_raw_field(field);
}
}
}

match cur.kind() {
CXCursor_FieldDecl => {
if let Some((ty, clang_ty, offset)) =
if let Some((ty, clang_ty, public, offset)) =
maybe_anonymous_struct_field.take()
{
let mut used = false;
Expand All @@ -1261,9 +1289,10 @@ impl CompInfo {
}
CXChildVisit_Continue
});

if !used {
let field = RawField::new(
None, ty, None, None, None, offset,
None, ty, None, None, None, public, offset,
);
ci.fields.append_raw_field(field);
}
Expand All @@ -1280,6 +1309,7 @@ impl CompInfo {
let comment = cur.raw_comment();
let annotations = Annotations::new(&cur);
let name = cur.spelling();
let is_public = cur.public_accessible();
let offset = cur.offset_of_field().ok();

// Name can be empty if there are bitfields, for example,
Expand All @@ -1297,6 +1327,7 @@ impl CompInfo {
comment,
annotations,
bit_width,
is_public,
offset,
);
ci.fields.append_raw_field(field);
Expand Down Expand Up @@ -1353,9 +1384,11 @@ impl CompInfo {
cur.kind() != CXCursor_EnumDecl
{
let ty = cur.cur_type();
let public = cur.public_accessible();
let offset = cur.offset_of_field().ok();

maybe_anonymous_struct_field =
Some((inner, ty, offset));
Some((inner, ty, public, offset));
}
}
CXCursor_PackedAttr => {
Expand Down Expand Up @@ -1388,6 +1421,8 @@ impl CompInfo {
ty: type_id,
kind,
field_name,
is_pub: cur.access_specifier() ==
clang_sys::CX_CXXPublic,
});
}
CXCursor_Constructor | CXCursor_Destructor |
Expand Down Expand Up @@ -1503,8 +1538,9 @@ impl CompInfo {
CXChildVisit_Continue
});

if let Some((ty, _, offset)) = maybe_anonymous_struct_field {
let field = RawField::new(None, ty, None, None, None, offset);
if let Some((ty, _, public, offset)) = maybe_anonymous_struct_field {
let field =
RawField::new(None, ty, None, None, None, public, offset);
ci.fields.append_raw_field(field);
}

Expand Down
15 changes: 15 additions & 0 deletions src/lib.rs
View file
Open in desktop
Expand Up @@ -545,6 +545,10 @@ impl Builder {
output_vector.push(name.clone());
}

if self.options.respect_cxx_access_specs {
output_vector.push("--respect-cxx-access-specs".into());
}

// Add clang arguments

output_vector.push("--".into());
Expand Down Expand Up @@ -1518,6 +1522,12 @@ impl Builder {
self.options.dynamic_library_name = Some(dynamic_library_name.into());
self
}

/// Generate bindings as `pub` only if the bound item is publically accessible by C++.
pub fn respect_cxx_access_specs(mut self, doit: bool) -> Self {
self.options.respect_cxx_access_specs = doit;
self
}
}

/// Configuration options for generated bindings.
Expand Down Expand Up @@ -1805,6 +1815,10 @@ struct BindgenOptions {
/// The name of the dynamic library (if we are generating bindings for a shared library). If
/// this is None, no dynamic bindings are created.
dynamic_library_name: Option<String>,

/// Only make generated bindings `pub` if the items would be publically accessible
/// by C++.
respect_cxx_access_specs: bool,
}

/// TODO(emilio): This is sort of a lie (see the error message that results from
Expand Down Expand Up @@ -1941,6 +1955,7 @@ impl Default for BindgenOptions {
array_pointers_in_arguments: false,
wasm_import_module_name: None,
dynamic_library_name: None,
respect_cxx_access_specs: false,
}
}
}
Expand Down