Skip to content

Commit

Permalink
Auto merge of #1312 - emilio:constness-woes, r=fitzgen
Browse files Browse the repository at this point in the history
ir: Handle *const T at the codegen level.

Followup to #1311.
  • Loading branch information
bors-servo authored May 15, 2018
2 parents 74dcb20 + 716d53b commit d6d097c
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3054,7 +3054,7 @@ impl TryToRustTy for Type {
}
TypeKind::Pointer(inner) |
TypeKind::Reference(inner) => {
let is_const = self.is_const() || ctx.resolve_type(inner).is_const();
let is_const = ctx.resolve_type(inner).is_const();

let inner = inner.into_resolver().through_type_refs().resolve(ctx);
let inner_ty = inner.expect_type();
Expand Down
37 changes: 36 additions & 1 deletion src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1925,9 +1925,44 @@ impl BindgenContext {
wrapped_id: TypeId,
parent_id: Option<ItemId>,
ty: &clang::Type,
) -> TypeId {
self.build_wrapper(
with_id,
wrapped_id,
parent_id,
ty,
ty.is_const(),
)
}

/// A wrapper over a type that adds a const qualifier explicitly.
///
/// Needed to handle const methods in C++, wrapping the type .
pub fn build_const_wrapper(
&mut self,
with_id: ItemId,
wrapped_id: TypeId,
parent_id: Option<ItemId>,
ty: &clang::Type,
) -> TypeId {
self.build_wrapper(
with_id,
wrapped_id,
parent_id,
ty,
/* is_const = */ true,
)
}

fn build_wrapper(
&mut self,
with_id: ItemId,
wrapped_id: TypeId,
parent_id: Option<ItemId>,
ty: &clang::Type,
is_const: bool,
) -> TypeId {
let spelling = ty.spelling();
let is_const = ty.is_const();
let layout = ty.fallible_layout().ok();
let type_kind = TypeKind::ResolvedTypeRef(wrapped_id);
let ty = Type::new(Some(spelling), layout, type_kind, is_const);
Expand Down
17 changes: 15 additions & 2 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,14 +402,27 @@ impl FunctionSig {
let is_virtual = is_method && cursor.method_is_virtual();
let is_static = is_method && cursor.method_is_static();
if !is_static && !is_virtual {
let class = Item::parse(cursor.semantic_parent(), None, ctx)
let parent = cursor.semantic_parent();
let class = Item::parse(parent, None, ctx)
.expect("Expected to parse the class");
// The `class` most likely is not finished parsing yet, so use
// the unchecked variant.
let class = class.as_type_id_unchecked();

let class = if is_const {
let const_class_id = ctx.next_item_id();
ctx.build_const_wrapper(
const_class_id,
class,
None,
&parent.cur_type(),
)
} else {
class
};

let ptr =
Item::builtin_type(TypeKind::Pointer(class), is_const, ctx);
Item::builtin_type(TypeKind::Pointer(class), false, ctx);
args.insert(0, (Some("this".into()), ptr));
} else if is_virtual {
let void = Item::builtin_type(TypeKind::Void, false, ctx);
Expand Down
4 changes: 2 additions & 2 deletions src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1449,8 +1449,8 @@ impl ClangItemParser for Item {
return Ok(Item::new_opaque_type(id, ty, ctx));
}

if let Some(id) = Item::type_param(Some(id), location, ctx) {
return Ok(id);
if let Some(param_id) = Item::type_param(None, location, ctx) {
return Ok(ctx.build_ty_wrapper(id, param_id, None, ty));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/ir/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ impl Opaque {
pub fn from_clang_ty(ty: &clang::Type) -> Type {
let layout = Layout::new(ty.size(), ty.align());
let ty_kind = TypeKind::Opaque;
Type::new(None, Some(layout), ty_kind, false)
let is_const = ty.is_const();
Type::new(None, Some(layout), ty_kind, is_const)
}

/// Return the known rust type we should use to create a correctly-aligned
Expand Down
17 changes: 1 addition & 16 deletions src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1195,22 +1195,7 @@ impl Type {

let name = if name.is_empty() { None } else { Some(name) };

// Just using ty.is_const() is wrong here, because when we declare an
// argument like 'int* const arg0', arg0 is considered
// const but the pointer itself points to mutable data.
//
// Without canonicalizing the type to the pointer type, we'll get the
// following mapping:
//
// arg0: *const c_int
//
// So by canonicalizing the type first, we can check constness by
// calling is_const() on the pointer type.
let is_const = if let Some(pty) = ty.pointee_type() {
pty.is_const()
} else {
ty.is_const()
};
let is_const = ty.is_const();

let ty = Type::new(name, layout, kind, is_const);
// TODO: maybe declaration.canonical()?
Expand Down
32 changes: 32 additions & 0 deletions tests/expectations/tests/const-const-mut-ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* automatically generated by rust-bindgen */

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

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct foo {
pub bar: *const *const *mut *const ::std::os::raw::c_int,
}
#[test]
fn bindgen_test_layout_foo() {
assert_eq!(
::std::mem::size_of::<foo>(),
8usize,
concat!("Size of: ", stringify!(foo))
);
assert_eq!(
::std::mem::align_of::<foo>(),
8usize,
concat!("Alignment of ", stringify!(foo))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<foo>())).bar as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(foo), "::", stringify!(bar))
);
}
impl Default for foo {
fn default() -> Self {
unsafe { ::std::mem::zeroed() }
}
}
3 changes: 3 additions & 0 deletions tests/headers/const-const-mut-ptr.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
struct foo {
const int** const* const* bar;
};

0 comments on commit d6d097c

Please sign in to comment.