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

Follow multiple transparent links in Option layout #699

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
11 changes: 8 additions & 3 deletions src/bindgen/ir/function.rs
Expand Up @@ -17,6 +17,7 @@ use crate::bindgen::library::Library;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::rename::{IdentifierType, RenameRule};
use crate::bindgen::reserved;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::utilities::IterHelpers;
use crate::bindgen::writer::{Source, SourceWriter};

Expand Down Expand Up @@ -127,10 +128,14 @@ impl Function {
&self.path
}

pub fn simplify_standard_types(&mut self, config: &Config) {
self.ret.simplify_standard_types(config);
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
self.ret.simplify_standard_types(config, transparent_types);
for arg in &mut self.args {
arg.ty.simplify_standard_types(config);
arg.ty.simplify_standard_types(config, transparent_types);
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/bindgen/ir/global.rs
Expand Up @@ -10,6 +10,7 @@ use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver;
use crate::bindgen::dependencies::Dependencies;
use crate::bindgen::ir::{AnnotationSet, Cfg, Documentation, Item, ItemContainer, Path, Type};
use crate::bindgen::library::Library;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::writer::{Source, SourceWriter};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -61,8 +62,12 @@ impl Static {
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
self.ty.simplify_standard_types(config);
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
self.ty.simplify_standard_types(config, transparent_types);
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/bindgen/ir/structure.rs
Expand Up @@ -16,6 +16,7 @@ use crate::bindgen::mangle;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::rename::{IdentifierType, RenameRule};
use crate::bindgen::reserved;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::utilities::IterHelpers;
use crate::bindgen::writer::{ListType, Source, SourceWriter};

Expand Down Expand Up @@ -142,9 +143,13 @@ impl Struct {
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
for field in &mut self.fields {
field.ty.simplify_standard_types(config);
field.ty.simplify_standard_types(config, transparent_types);
}
}

Expand Down
32 changes: 27 additions & 5 deletions src/bindgen/ir/ty.rs
Expand Up @@ -12,6 +12,7 @@ use crate::bindgen::dependencies::Dependencies;
use crate::bindgen::ir::{GenericParams, GenericPath, Path};
use crate::bindgen::library::Library;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::utilities::IterHelpers;
use crate::bindgen::writer::{Source, SourceWriter};

Expand Down Expand Up @@ -588,7 +589,11 @@ impl Type {
}))
}

fn simplified_type(&self, config: &Config) -> Option<Self> {
fn simplified_type(
&self,
config: &Config,
transparent_types: &TransparentTypes,
) -> Option<Self> {
let path = match *self {
Type::Path(ref p) => p,
_ => return None,
Expand All @@ -603,12 +608,25 @@ impl Type {
}

let unsimplified_generic = &path.generics()[0];
let generic = match unsimplified_generic.simplified_type(config) {
let mut generic = match unsimplified_generic.simplified_type(config, transparent_types) {
Some(generic) => Cow::Owned(generic),
None => Cow::Borrowed(unsimplified_generic),
};
match path.name() {
"Option" => {
// Repeatedly follow any typedefs or transparent structs until we reach a type that
// has its own "real" type in C/C++.
while let Some(transparent) = transparent_types.is_transparent(&generic) {
// Make sure to do another round of simplifying each time we follow a typedef
// or transparent struct link.
generic = match transparent.simplified_type(config, transparent_types) {
Some(generic) => Cow::Owned(generic),
None => Cow::Owned(transparent),
};
}

// Once we've found the base type for the Option's generic, see if we can take
// advantage of any nullable/zeroable layout optimizations.
if let Some(nullable) = generic.make_nullable() {
return Some(nullable);
}
Expand Down Expand Up @@ -637,9 +655,13 @@ impl Type {
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
self.visit_types(|ty| ty.simplify_standard_types(config));
if let Some(ty) = self.simplified_type(config) {
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
self.visit_types(|ty| ty.simplify_standard_types(config, transparent_types));
if let Some(ty) = self.simplified_type(config, transparent_types) {
*self = ty;
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/bindgen/ir/typedef.rs
Expand Up @@ -15,6 +15,7 @@ use crate::bindgen::ir::{
use crate::bindgen::library::Library;
use crate::bindgen::mangle;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::writer::{Source, SourceWriter};

/// A type alias that is represented as a C typedef
Expand Down Expand Up @@ -66,8 +67,13 @@ impl Typedef {
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
self.aliased.simplify_standard_types(config);
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
self.aliased
.simplify_standard_types(config, transparent_types);
}

pub fn transfer_annotations(&mut self, out: &mut HashMap<Path, AnnotationSet>) {
Expand Down
9 changes: 7 additions & 2 deletions src/bindgen/ir/union.rs
Expand Up @@ -15,6 +15,7 @@ use crate::bindgen::library::Library;
use crate::bindgen::mangle;
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::rename::{IdentifierType, RenameRule};
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::utilities::IterHelpers;
use crate::bindgen::writer::{ListType, Source, SourceWriter};

Expand Down Expand Up @@ -95,9 +96,13 @@ impl Union {
}
}

pub fn simplify_standard_types(&mut self, config: &Config) {
pub fn simplify_standard_types(
&mut self,
config: &Config,
transparent_types: &TransparentTypes,
) {
for field in &mut self.fields {
field.ty.simplify_standard_types(config);
field.ty.simplify_standard_types(config, transparent_types);
}
}

Expand Down
14 changes: 9 additions & 5 deletions src/bindgen/library.rs
Expand Up @@ -12,6 +12,7 @@ use crate::bindgen::error::Error;
use crate::bindgen::ir::{Constant, Enum, Function, Item, ItemContainer, ItemMap};
use crate::bindgen::ir::{OpaqueItem, Path, Static, Struct, Typedef, Union};
use crate::bindgen::monomorph::Monomorphs;
use crate::bindgen::transparent_types::TransparentTypes;
use crate::bindgen::ItemType;

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -358,21 +359,24 @@ impl Library {

fn simplify_standard_types(&mut self) {
let config = &self.config;
let mut transparent_types = TransparentTypes::default();
transparent_types.add_structs(&self.structs);
transparent_types.add_typedefs(&self.typedefs);

self.structs.for_all_items_mut(|x| {
x.simplify_standard_types(config);
x.simplify_standard_types(config, &transparent_types);
});
self.unions.for_all_items_mut(|x| {
x.simplify_standard_types(config);
x.simplify_standard_types(config, &transparent_types);
});
self.globals.for_all_items_mut(|x| {
x.simplify_standard_types(config);
x.simplify_standard_types(config, &transparent_types);
});
self.typedefs.for_all_items_mut(|x| {
x.simplify_standard_types(config);
x.simplify_standard_types(config, &transparent_types);
});
for x in &mut self.functions {
x.simplify_standard_types(config);
x.simplify_standard_types(config, &transparent_types);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/bindgen/mod.rs
Expand Up @@ -52,6 +52,7 @@ mod monomorph;
mod parser;
mod rename;
mod reserved;
mod transparent_types;
mod utilities;
mod writer;

Expand Down
49 changes: 49 additions & 0 deletions src/bindgen/transparent_types.rs
@@ -0,0 +1,49 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use std::collections::HashMap;

use crate::bindgen::ir::{GenericParams, ItemMap, Path, Struct, Type, Typedef};

/// Keeps track of named types that have the same underlying representation as some other type.
/// This happens via `#[repr(transparent)]` structs and via typedefs.
#[derive(Default)]
pub struct TransparentTypes {
transparent: HashMap<Path, (Type, GenericParams)>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

So it is quite a bit unfortunate to have to go through all items again creating this data structure when we already have a <Path, Struct> etc hashmap in ItemMap, it just feels like a lot of upfront work for something that could be much more efficient.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I definitely agree. At first I tried passing &ItemMap<Struct> and &ItemMap<Typedef> references in to the simplify_standard_types methods, but that runs afoul of the borrow checker, since we're calling them from within a for_all_items_mut loop that is updating the contents of those ItemMaps. Do you have any suggestions for how to break that double-borrow?

}

impl TransparentTypes {
pub fn add_structs(&mut self, structs: &ItemMap<Struct>) {
structs.for_all_items(|s| {
if s.is_transparent {
self.transparent.insert(
s.path.clone(),
(s.fields[0].ty.clone(), s.generic_params.clone()),
);
}
});
}

pub fn add_typedefs(&mut self, structs: &ItemMap<Typedef>) {
structs.for_all_items(|t| {
self.transparent.insert(
t.path.clone(),
(t.aliased.clone(), t.generic_params.clone()),
);
});
}

pub fn is_transparent(&self, ty: &Type) -> Option<Type> {
let generic_path = match ty {
Type::Path(p) => p,
_ => return None,
};
let (resolved, generic_params) = self.transparent.get(generic_path.path())?;
let mappings = generic_params
.iter()
.zip(generic_path.generics())
.collect::<Vec<_>>();
Some(resolved.specialize(&mappings))
}
}
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.both.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

typedef struct Bar {
Option_Foo foo;
uint64_t foo;
} Bar;

void root(struct Bar f);
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.both.compat.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

typedef struct Bar {
Option_Foo foo;
uint64_t foo;
} Bar;

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

typedef struct {
Option_Foo foo;
uint64_t foo;
} Bar;

void root(Bar f);
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.compat.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

typedef struct {
Option_Foo foo;
uint64_t foo;
} Bar;

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.cpp
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

typedef struct Bar {
Option_Foo foo;
uint64_t foo;
} Bar;

void root(struct Bar f);
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.pyx
Expand Up @@ -25,6 +25,6 @@ cdef extern from *:
cdef extern from *:

ctypedef struct Bar:
Option_Foo foo;
uint64_t foo;

void root(Bar f);
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.tag.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

struct Bar {
Option_Foo foo;
uint64_t foo;
};

void root(struct Bar f);
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.tag.compat.c
Expand Up @@ -22,7 +22,7 @@ ctypedef uint64_t Option_Foo
#include <stdlib.h>

struct Bar {
Option_Foo foo;
uint64_t foo;
};

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion tests/expectations/exclude_generic_monomorph.tag.pyx
Expand Up @@ -25,6 +25,6 @@ cdef extern from *:
cdef extern from *:

cdef struct Bar:
Option_Foo foo;
uint64_t foo;

void root(Bar f);
21 changes: 21 additions & 0 deletions tests/expectations/nested_nonnull.both.c
@@ -0,0 +1,21 @@
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

typedef int32_t (*DoFn)(int32_t x, int32_t y);

typedef struct StructWithOptionalFunctionPointer {
DoFn func;
int32_t (*maybe_func)(int32_t x, int32_t y);
} StructWithOptionalFunctionPointer;

typedef uint32_t *NonNullAlias_u32;

typedef struct StructWithOptionalNonNullPointer {
NonNullAlias_u32 data;
uint32_t *maybe_data;
} StructWithOptionalNonNullPointer;

void root(struct StructWithOptionalFunctionPointer swofp,
struct StructWithOptionalNonNullPointer swonnp);