Skip to content

Unions: Remove feature-gate for stable rustc #415

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

Merged
merged 5 commits into from
Feb 1, 2018
Merged

Unions: Remove feature-gate for stable rustc #415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
21a7e4f
Unions: Remove feature-gate for stable rustc
Jul 21, 2017
a5e2bbb
Unions: make all structs and unions copy,clone
Jul 29, 2017
aeefc4d
Unions: tweaks to copy,clone, and remove unneeded if/else
Jul 29, 2017
8560db7
unions: rebase & fix copy,clone for unions
Nov 13, 2017
0e6665f
tests: update some crate versions, eg, bitflags
Nov 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,3 @@ git2 = { version = "0.6", default-features = false }

[profile.release]
codegen-units = 4

[features]
default = []
use_unions = []
4 changes: 1 addition & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,9 @@ It'll generate a `docs.md` if everything went fine. That's where all this crate'

And now your crate should be completely documented as expected!

## Nightly Rust Only Features

### Unions

By default union generation is disabled except for some special cases due to unions not yet being a stable feature. However if you are using *nightly* rust, then you can enable union generation using `cargo run --release --features "use_unions"`.
`gir` now has the ability to generate c-like unions using newly stabilised `union` in rustc 1.19. As such this means `gir` requires a minimum version rustc of 1.19

Keep in mind that to access union members, you are required to use `unsafe` blocks, for example;

Expand Down
161 changes: 55 additions & 106 deletions src/codegen/sys/lib_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,67 +317,34 @@ fn generate_unions(w: &mut Write, env: &Env, items: &[&Union]) -> Result<()> {

for item in items {
if let Some(ref c_type) = item.c_type {
#[cfg(feature = "use_unions")]
{
let (lines, commented) = generate_fields(env, &item.name, &item.fields);

let comment = if commented { "//" } else { "" };
if lines.is_empty() {
try!(writeln!(
w,
"{comment}#[repr(C)]\n{comment}pub union {name}(c_void);\n",
comment = comment,
name = c_type
));
} else {
try!(writeln!(
w,
"{comment}#[repr(C)]\n{comment}pub union {name} {{",
comment = comment,
name = c_type
));

for line in lines {
try!(writeln!(w, "{}{}", comment, line));
}
try!(writeln!(w, "{}}}\n", comment));
}
if comment.is_empty() {
try!(generate_debug_with_fields(w, name, &lines));
let (lines, commented) = generate_fields(env, &item.name, &item.fields);

let comment = if commented { "//" } else { "" };
if lines.is_empty() {
try!(writeln!(
w,
"{0}#[repr(C)]\n{0}#[derive(Copy,Clone,Debug)]\n{0}pub union {1}(u8);\n",
comment,
c_type
));
} else {
try!(writeln!(
w,
"{0}#[repr(C)]\n{0}#[derive(Copy,Clone)]\n{0}pub union {1} {{",
comment,
c_type
));

for line in &lines {
try!(writeln!(w, "{}{}", comment, line));
}
try!(writeln!(w, "{}}}\n", comment));
}
#[cfg(not(feature = "use_unions"))]
{
// TODO: GLib/GObject special cases until we have proper union support in Rust
if env.config.library_name == "GLib" && c_type == "GMutex" {
// Two c_uint or a pointer => 64 bits on all platforms currently
// supported by GLib but the alignment is different on 32 bit
// platforms (32 bit vs. 64 bits on 64 bit platforms)
try!(writeln!(
w,
"#[cfg(target_pointer_width = \"32\")]\n\
#[repr(C)]\n\
#[derive(Debug)]\n\
pub struct {0}([u32; 2]);\n\
#[cfg(target_pointer_width = \"64\")]\n\
#[repr(C)]\n\
#[derive(Debug)]\n\
pub struct {0}(*mut c_void);",
c_type
));
} else {
try!(writeln!(w, "pub type {} = c_void; // union", c_type));
}
if comment.is_empty() {
try!(generate_debug_with_fields(w, c_type, &lines, false));
}
}
}
#[cfg(not(feature = "use_unions"))]
{
if !items.is_empty() {
try!(writeln!(w, ""));
}
}

Ok(())
}

Expand All @@ -393,7 +360,7 @@ fn generate_debug_impl(w: &mut Write, name: &str, impl_content: &str) -> Result<
impl_content)
}

fn generate_debug_with_fields(w: &mut Write, name: &str, lines: &[String]) -> Result<()> {
fn generate_debug_with_fields(w: &mut Write, name: &str, lines: &[String], safe: bool) -> Result<()> {
let fields =
lines.iter()
.filter_map(|field| {
Expand All @@ -413,12 +380,23 @@ fn generate_debug_with_fields(w: &mut Write, name: &str, lines: &[String]) -> Re
generate_debug_impl(
w,
name,
&format!("f.debug_struct(&format!(\"{name} @ {{:?}}\", self as *const _))\n\
&if safe {
format!("f.debug_struct(&format!(\"{name} @ {{:?}}\", self as *const _))\n\
{fields}\
\t\t .finish()",
name=name,
fields=fields,
)
name=name,
fields=fields,
)
} else {
format!("unsafe {{\n\
\t\t f.debug_struct(&format!(\"{name} @ {{:?}}\", self as *const _))\n\
{fields}\
\t\t .finish()\n\
\t\t }}",
name=name,
fields=fields,
)
},
)
}

Expand Down Expand Up @@ -449,7 +427,8 @@ fn generate_classes_structs(w: &mut Write, env: &Env, classes: &[&Class]) -> Res
w,
"{comment}#[repr(C)]\n{debug}{comment}pub struct {name} {{",
comment = comment,
debug = if can_generate_fields_debug { "#[derive(Debug)]\n" } else { "" },
debug = if can_generate_fields_debug { "#[derive(Copy,Clone,Debug)]\n" }
else { "#[derive(Copy,Clone)]\n" },
name = klass.c_type,
));

Expand All @@ -459,7 +438,7 @@ fn generate_classes_structs(w: &mut Write, env: &Env, classes: &[&Class]) -> Res
try!(writeln!(w, "{}}}\n", comment));

if !can_generate_fields_debug && comment.is_empty() {
try!(generate_debug_with_fields(w, &klass.c_type, &lines));
try!(generate_debug_with_fields(w, &klass.c_type, &lines, true));
}
}
}
Expand Down Expand Up @@ -511,7 +490,7 @@ fn generate_records(w: &mut Write, env: &Env, records: &[&Record]) -> Result<()>
if lines.is_empty() {
try!(writeln!(
w,
"{}#[repr(C)]\n{0}pub struct {}(c_void);\n",
"{0}#[repr(C)]\n{0}#[derive(Copy,Clone)]\n{0}pub struct {1}(u8);\n",
Copy link
Member

@EPashkin EPashkin Jul 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This better changed back (it will produce only 1 extra "dont_copy")

Copy link
Member

Choose a reason for hiding this comment

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

..because it just don't right, copy record with unknown size

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure I understand, pub struct GIConv(u8); should not be copy?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, with GIOChannel

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand either. What is the broken code generated by this?

Copy link
Member

Choose a reason for hiding this comment

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

this case produce warning: found non-foreign-function-safe member in struct marked #[repr(C)]: found Rust tuple type in foreign module; consider using a struct instead
just struct produce warning: found zero-size struct in foreign module, consider adding a member to this struct

Copy link
Member

Choose a reason for hiding this comment

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

I see... a case for the new #[repr(transparent)] I guess

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, now I don't understand how it related to struct without fields.

Copy link
Member

Choose a reason for hiding this comment

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

You would want to represent an opaque C type. It's not really a struct without fields, it's just that the struct definition is unknown from the headers. So you can only ever use it as a pointer.

But that's actually extern type: rust-lang/rfcs#1861

Copy link
Member

Choose a reason for hiding this comment

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

extern type is that we need for this case but it not realized and may cause build error when inserted in other struct directly (as with GIConv)
#[repr(transparent)] can be used for minimize alignment errors for known fields, so also not for this case, so last repr(C).
I prefer c_void over u8 as it don't implement Copy while has same size and alignment.

comment,
record.c_type
));
Expand All @@ -531,7 +510,8 @@ fn generate_records(w: &mut Write, env: &Env, records: &[&Record]) -> Result<()>
w,
"{}#[repr(C)]\n{}{0}pub struct {} {{",
comment,
if can_generate_fields_debug(&record.fields) { "#[derive(Debug)]\n" } else { "" },
if can_generate_fields_debug(&record.fields) { "#[derive(Copy,Clone,Debug)]\n" }
else { "#[derive(Copy,Clone)]\n" },
record.c_type
));
for line in &lines {
Expand Down Expand Up @@ -590,43 +570,15 @@ fn generate_fields(env: &Env, struct_name: &str, fields: &[Field]) -> (Vec<Strin
}
};

// TODO: Special case for padding unions like used in GStreamer, see e.g.
// the padding in GstControlBinding
#[cfg(not(feature = "use_unions"))]
if !is_gweakref && !is_ghooklist && !truncated && !is_ptr && is_bits
{
if is_union && !truncated {
if let Some(union_) = env.library.type_(field.typ).maybe_ref_as::<Union>() {
for union_field in &union_.fields {
if union_field.name.contains("reserved")
|| union_field.name.contains("padding")
{
if let Some(ref c_type) = union_field.c_type {
let name = mangle_keywords(&*union_field.name);
let c_type = ffi_type(env, union_field.typ, c_type);
if c_type.is_err() {
commented = true;
}
lines.push(format!("\tpub {}: {},", name, c_type.into_string()));
continue 'fields;
}
}
}
}
}
}

if !cfg!(feature = "use_unions") || (is_bits && !truncated) {
if !is_gweakref && !is_ghooklist && !truncated && !is_ptr && (is_union || is_bits)
&& !is_union_special_case(&field.c_type)
{
warn!(
"Field `{}::{}` not expressible in Rust, truncated",
struct_name,
field.name
);
lines.push("\t_truncated_record_marker: c_void,".to_owned());
truncated = true;
}
warn!(
"Field `{}::{}` not expressible in Rust, truncated",
struct_name,
field.name
);
lines.push("\t_truncated_record_marker: u8,".to_owned());
truncated = true;
}

if truncated {
Expand Down Expand Up @@ -665,18 +617,15 @@ fn generate_fields(env: &Env, struct_name: &str, fields: &[Field]) -> (Vec<Strin
lines.push("\tpub hook_size_and_setup: c_ulong,".to_owned());
}
// is_setup is ignored
} else if is_gweakref && !cfg!(feature = "use_unions") {
} else if is_gweakref {
// union containing a single pointer
lines.push("\tpub priv_: gpointer,".to_owned());
} else if let Some(ref c_type) = field.c_type {
let name = mangle_keywords(&*field.name);
let mut c_type = ffi_type(env, field.typ, c_type);
let c_type = ffi_type(env, field.typ, c_type);
if c_type.is_err() {
commented = true;
}
if !cfg!(feature = "use_unions") && is_gvalue && field.name == "data" {
c_type = Ok("[u64; 2]".to_owned());
}
lines.push(format!("\tpub {}: {},", name, c_type.into_string()));
} else {
let name = mangle_keywords(&*field.name);
Expand Down
9 changes: 2 additions & 7 deletions src/codegen/sys/statics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,8 @@ use std::io::{Result, Write};
use super::super::general::write_vec;

pub fn begin(w: &mut Write) -> Result<()> {
#[cfg(feature = "use_unions")]
let u = "#![feature(untagged_unions)]";
#[cfg(not(feature = "use_unions"))]
let u = "";

let v = vec![
u,
"",
"#![allow(non_camel_case_types, non_upper_case_globals)]",
"",
"extern crate libc;",
Expand Down Expand Up @@ -52,7 +47,7 @@ pub fn only_for_glib(w: &mut Write) -> Result<()> {
"pub type gpointer = *mut c_void;",
"",
"#[repr(C)]",
"#[derive(Debug)]",
"#[derive(Copy,Clone,Debug)]",
"pub struct Volatile<T>(T);",
"",
];
Expand Down
49 changes: 15 additions & 34 deletions src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,7 @@ impl Library {
let ctype = u.c_type.clone();

let type_id = {
#[cfg(not(feature = "use_unions"))]
{
Type::union(self, u, INTERNAL_NAMESPACE)
}
#[cfg(feature = "use_unions")]
{
Type::union(self, u, ns_id)
}
Type::union(self, u, ns_id)
};

fields.push(Field {
Expand Down Expand Up @@ -436,14 +429,7 @@ impl Library {
let ctype = u.c_type.clone();

let type_id = {
#[cfg(not(feature = "use_unions"))]
{
Type::union(self, u, INTERNAL_NAMESPACE)
}
#[cfg(feature = "use_unions")]
{
Type::union(self, u, ns_id)
}
Type::union(self, u, ns_id)
};

fields.push(Field {
Expand Down Expand Up @@ -561,16 +547,22 @@ impl Library {
let f = try!(self.read_field(parser, ns_id, &attributes));
fields.push(f);
}
kind @ "constructor" | kind @ "function" | kind @ "method" => try!(
self.read_function_to_vec(parser, ns_id, kind, &attributes, &mut fns,)
),
kind @ "constructor" | kind @ "function" | kind @ "method" => {
try!(self.read_function_to_vec(
parser,
ns_id,
kind,
&attributes,
&mut fns,
))
}
"record" => {
let mut r = match try!(self.read_record(
parser,
ns_id,
attrs,
parent_name_prefix,
parent_ctype_prefix
parent_ctype_prefix,
)) {
Some(Type::Record(r)) => r,
_ => continue,
Expand All @@ -591,7 +583,7 @@ impl Library {
s.push('_');
s
})
.unwrap_or_else(String::new),
.unwrap_or("".into()),
union_name,
field_name
),
Expand All @@ -603,7 +595,7 @@ impl Library {
s.push('_');
s
})
.unwrap_or_else(String::new),
.unwrap_or("".into()),
c_type,
field_name
),
Expand All @@ -613,20 +605,9 @@ impl Library {
let r_doc = r.doc.clone();
let ctype = r.c_type.clone();

let type_id = {
#[cfg(not(feature = "use_unions"))]
{
Type::record(self, r, INTERNAL_NAMESPACE)
}
#[cfg(feature = "use_unions")]
{
Type::record(self, r, ns_id)
}
};

fields.push(Field {
name: field_name,
typ: type_id,
typ: Type::record(self, r, ns_id),
doc: r_doc,
c_type: Some(ctype),
..Field::default()
Expand Down
Loading