Skip to content

Commit

Permalink
ir: Avoid generating bogus pointer arguments.
Browse files Browse the repository at this point in the history 
Now that we track nullability in the pointer type it's easy to do it.

Fixes #223
  • Loading branch information
@emilio
emilio committed Jan 14, 2021
1 parent 4ba7d1f commit f922f68
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 30 deletions.
6 changes: 5 additions & 1 deletion src/bindgen/cdecl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,11 @@ impl CDecl {
self.declarators.push(CDeclarator::Array(len));
self.build_type(t, is_const, config);
}
Type::FuncPtr(ref ret, ref args) => {
Type::FuncPtr {
ref ret,
ref args,
is_nullable: _,
} => {
let args = args
.iter()
.map(|(ref name, ref ty)| (name.clone(), CDecl::from_type(ty, config)))
Expand Down
78 changes: 60 additions & 18 deletions src/bindgen/ir/ty.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,11 @@ pub enum Type {
Path(GenericPath),
Primitive(PrimitiveType),
Array(Box<Type>, ArrayLength),
FuncPtr(Box<Type>, Vec<(Option<String>, Type)>),
FuncPtr {
ret: Box<Type>,
args: Vec<(Option<String>, Type)>,
is_nullable: bool,
},
}

impl Type {
Expand Down Expand Up @@ -484,7 +488,11 @@ impl Type {
}
};

Type::FuncPtr(Box::new(ret), args)
Type::FuncPtr {
ret: Box::new(ret),
args,
is_nullable: false,
}
}
syn::Type::Tuple(ref tuple) => {
if tuple.elems.is_empty() {
Expand Down Expand Up @@ -515,14 +523,22 @@ impl Type {
ref ty,
is_const,
is_ref,
..
is_nullable: false,
} => Some(Type::Ptr {
ty: ty.clone(),
is_const,
is_ref,
is_nullable: true,
}),
Type::FuncPtr(ref x, ref y) => Some(Type::FuncPtr(x.clone(), y.clone())),
Type::FuncPtr {
ref ret,
ref args,
is_nullable: false,
} => Some(Type::FuncPtr {
ret: ret.clone(),
args: args.clone(),
is_nullable: true,
}),
_ => None,
}
}
Expand All @@ -543,7 +559,6 @@ impl Type {
None => Cow::Borrowed(unsimplified_generic),
};
match path.name() {
// FIXME(#223): This is not quite right.
"Option" => generic.make_nullable(),
"NonNull" => Some(Type::Ptr {
ty: Box::new(generic.into_owned()),
Expand Down Expand Up @@ -580,7 +595,11 @@ impl Type {
generic_path.replace_self_with(self_ty);
}
Type::Primitive(..) => {}
Type::FuncPtr(ref mut ret, ref mut args) => {
Type::FuncPtr {
ref mut ret,
ref mut args,
..
} => {
ret.replace_self_with(self_ty);
for arg in args {
arg.1.replace_self_with(self_ty);
Expand All @@ -603,7 +622,7 @@ impl Type {
Type::Array(..) => {
return None;
}
Type::FuncPtr(..) => {
Type::FuncPtr { .. } => {
return None;
}
};
Expand Down Expand Up @@ -644,13 +663,19 @@ impl Type {
Type::Array(ref ty, ref constant) => {
Type::Array(Box::new(ty.specialize(mappings)), constant.clone())
}
Type::FuncPtr(ref ret, ref args) => Type::FuncPtr(
Box::new(ret.specialize(mappings)),
args.iter()
Type::FuncPtr {
ref ret,
ref args,
is_nullable,
} => Type::FuncPtr {
ret: Box::new(ret.specialize(mappings)),
args: args
.iter()
.cloned()
.map(|(name, ty)| (name, ty.specialize(mappings)))
.collect(),
),
is_nullable,
},
}
}

Expand Down Expand Up @@ -694,7 +719,9 @@ impl Type {
Type::Array(ref ty, _) => {
ty.add_dependencies_ignoring_generics(generic_params, library, out);
}
Type::FuncPtr(ref ret, ref args) => {
Type::FuncPtr {
ref ret, ref args, ..
} => {
ret.add_dependencies_ignoring_generics(generic_params, library, out);
for (_, ref arg) in args {
arg.add_dependencies_ignoring_generics(generic_params, library, out);
Expand Down Expand Up @@ -728,7 +755,9 @@ impl Type {
Type::Array(ref ty, _) => {
ty.add_monomorphs(library, out);
}
Type::FuncPtr(ref ret, ref args) => {
Type::FuncPtr {
ref ret, ref args, ..
} => {
ret.add_monomorphs(library, out);
for (_, ref arg) in args {
arg.add_monomorphs(library, out);
Expand All @@ -750,7 +779,11 @@ impl Type {
ty.rename_for_config(config, generic_params);
len.rename_for_config(config);
}
Type::FuncPtr(ref mut ret, ref mut args) => {
Type::FuncPtr {
ref mut ret,
ref mut args,
..
} => {
ret.rename_for_config(config, generic_params);
for (_, arg) in args {
arg.rename_for_config(config, generic_params);
Expand All @@ -771,7 +804,11 @@ impl Type {
Type::Array(ref mut ty, _) => {
ty.resolve_declaration_types(resolver);
}
Type::FuncPtr(ref mut ret, ref mut args) => {
Type::FuncPtr {
ref mut ret,
ref mut args,
..
} => {
ret.resolve_declaration_types(resolver);
for (_, ref mut arg) in args {
arg.resolve_declaration_types(resolver);
Expand Down Expand Up @@ -804,7 +841,11 @@ impl Type {
Type::Array(ref mut ty, _) => {
ty.mangle_paths(monomorphs);
}
Type::FuncPtr(ref mut ret, ref mut args) => {
Type::FuncPtr {
ref mut ret,
ref mut args,
..
} => {
ret.mangle_paths(monomorphs);
for (_, ref mut arg) in args {
arg.mangle_paths(monomorphs);
Expand All @@ -815,11 +856,12 @@ impl Type {

pub fn can_cmp_order(&self) -> bool {
match *self {
// FIXME: Shouldn't this look at ty.can_cmp_order() as well?
Type::Ptr { is_ref, .. } => !is_ref,
Type::Path(..) => true,
Type::Primitive(ref p) => p.can_cmp_order(),
Type::Array(..) => false,
Type::FuncPtr(..) => false,
Type::FuncPtr { .. } => false,
}
}

Expand All @@ -829,7 +871,7 @@ impl Type {
Type::Path(..) => true,
Type::Primitive(ref p) => p.can_cmp_eq(),
Type::Array(..) => false,
Type::FuncPtr(..) => true,
Type::FuncPtr { .. } => true,
}
}
}
Expand Down
19 changes: 18 additions & 1 deletion src/bindgen/mangle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ enum Separator {
ClosingAngleBracket,
BeginMutPtr,
BeginConstPtr,
BeginFn,
BetweenFnArg,
EndFn,
}

struct Mangler<'a> {
Expand Down Expand Up @@ -93,7 +96,21 @@ impl<'a> Mangler<'a> {
});
self.append_mangled_type(&**ty, last);
}
Type::Array(..) | Type::FuncPtr(..) => {
Type::FuncPtr {
ref ret, ref args, ..
} => {
self.push(Separator::BeginFn);
self.append_mangled_type(&**ret, args.is_empty());
for (i, arg) in args.iter().enumerate() {
self.push(Separator::BetweenFnArg);
let last = last && i == args.len() - 1;
self.append_mangled_type(&arg.1, last);
}
if !self.last {
self.push(Separator::EndFn);
}
}
Type::Array(..) => {
unimplemented!(
"Unable to mangle generic parameter {:?} for '{}'",
ty,
Expand Down
12 changes: 11 additions & 1 deletion tests/expectations/simplify_option_ptr.both.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,26 @@

typedef struct Opaque Opaque;

typedef struct Option_____Opaque Option_____Opaque;

typedef struct Option_______c_void Option_______c_void;

typedef struct Foo {
const struct Opaque *x;
struct Opaque *y;
void (*z)(void);
struct Option_______c_void *zz;
} Foo;

typedef union Bar {
const struct Opaque *x;
struct Opaque *y;
void (*z)(void);
struct Option_______c_void *zz;
} Bar;

void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d);
void root(const struct Opaque *a,
struct Opaque *b,
struct Foo c,
union Bar d,
struct Option_____Opaque *e);
12 changes: 11 additions & 1 deletion tests/expectations/simplify_option_ptr.both.compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,33 @@

typedef struct Opaque Opaque;

typedef struct Option_____Opaque Option_____Opaque;

typedef struct Option_______c_void Option_______c_void;

typedef struct Foo {
const struct Opaque *x;
struct Opaque *y;
void (*z)(void);
struct Option_______c_void *zz;
} Foo;

typedef union Bar {
const struct Opaque *x;
struct Opaque *y;
void (*z)(void);
struct Option_______c_void *zz;
} Bar;

#ifdef __cplusplus
extern "C" {
#endif // __cplusplus

void root(const struct Opaque *a, struct Opaque *b, struct Foo c, union Bar d);
void root(const struct Opaque *a,
struct Opaque *b,
struct Foo c,
union Bar d,
struct Option_____Opaque *e);

#ifdef __cplusplus
} // extern "C"
Expand Down
8 changes: 7 additions & 1 deletion tests/expectations/simplify_option_ptr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,22 @@

typedef struct Opaque Opaque;

typedef struct Option_____Opaque Option_____Opaque;

typedef struct Option_______c_void Option_______c_void;

typedef struct {
const Opaque *x;
Opaque *y;
void (*z)(void);
Option_______c_void *zz;
} Foo;

typedef union {
const Opaque *x;
Opaque *y;
void (*z)(void);
Option_______c_void *zz;
} Bar;

void root(const Opaque *a, Opaque *b, Foo c, Bar d);
void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e);
8 changes: 7 additions & 1 deletion tests/expectations/simplify_option_ptr.compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,29 @@

typedef struct Opaque Opaque;

typedef struct Option_____Opaque Option_____Opaque;

typedef struct Option_______c_void Option_______c_void;

typedef struct {
const Opaque *x;
Opaque *y;
void (*z)(void);
Option_______c_void *zz;
} Foo;

typedef union {
const Opaque *x;
Opaque *y;
void (*z)(void);
Option_______c_void *zz;
} Bar;

#ifdef __cplusplus
extern "C" {
#endif // __cplusplus

void root(const Opaque *a, Opaque *b, Foo c, Bar d);
void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option_____Opaque *e);

#ifdef __cplusplus
} // extern "C"
Expand Down
7 changes: 6 additions & 1 deletion tests/expectations/simplify_option_ptr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,25 @@

struct Opaque;

template<typename T = void>
struct Option;

struct Foo {
const Opaque *x;
Opaque *y;
void (*z)();
Option<void(*)()> *zz;
};

union Bar {
const Opaque *x;
Opaque *y;
void (*z)();
Option<void(*)()> *zz;
};

extern "C" {

void root(const Opaque *a, Opaque *b, Foo c, Bar d);
void root(const Opaque *a, Opaque *b, Foo c, Bar d, Option<Opaque*> *e);

} // extern "C"
Loading

0 comments on commit f922f68

Please sign in to comment.