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

Generates un-Copyable structs with #[derive(Copy)] in case of padding #534

Closed
sdroege opened this issue Feb 6, 2018 · 15 comments
Closed

Comments

@sdroege
Copy link
Member

sdroege commented Feb 6, 2018

In GStreamer for example:

#[derive(Copy, Clone)]
pub struct GstVideoCodecState {
    pub ref_count: c_int,
    pub info: GstVideoInfo,
    pub caps: *mut gst::GstCaps,
    pub codec_data: *mut gst::GstBuffer,
    pub allocation_caps: *mut gst::GstCaps,
    pub padding: [c_void; 19],
}

leads to

error[E0204]: the trait `Copy` may not be implemented for this type
   --> gstreamer-video-sys/src/lib.rs:817:10
    |
817 | #[derive(Copy, Clone)]
    |          ^^^^
...
824 |     pub padding: [c_void; 19],
    |     ------------------------- this field does not implement `Copy`

Problem here is (AFAIU) that this is c_void, it should be *mut c_void or gpointer.

@sdroege
Copy link
Member Author

sdroege commented Feb 6, 2018

@tmiasko @EPashkin Any ideas here? The field in question is

      <field name="padding" readable="0" private="1">
        <array zero-terminated="0" c:type="void" fixed-size="19">
          <type name="gpointer" c:type="void*"/>
        </array>
      </field>

In other places where it works, it is

      <field name="_gst_reserved" readable="0" private="1">
        <array zero-terminated="0" c:type="gpointer" fixed-size="3">
          <type name="gpointer" c:type="gpointer"/>
        </array>
      </field>

@sdroege
Copy link
Member Author

sdroege commented Feb 6, 2018

Changing void * in the C header to gpointer would be fixing this, but that does not seem like a good solution

@sdroege
Copy link
Member Author

sdroege commented Feb 6, 2018

This is the same as #478, but more severe now as things stop compiling, while before it was only a problem with the struct size.

@ghost
Copy link

ghost commented Feb 6, 2018

Apart from the fact that this is bug in gir xml, see #478 (comment),
I think on our part, we could do better by always looking at the innermost
c:type attribute. It seems to be the most accurate. This is somewhat tricky
because in case of arrays we don't retain it after parsing.

@EPashkin
Copy link
Member

EPashkin commented Feb 6, 2018

We use field "c_type" too aggressively in sys,
Seems only way in code is check for c_type for Some("void") and change it to Some("gpointer") before https://github.com/gtk-rs/gir/blob/master/src/parser.rs#L1165
Or just do same replace for all 5 c:type="void" fixed-size= in GstVideo-1.0.gir

@EPashkin
Copy link
Member

EPashkin commented Feb 6, 2018

Inner type stored in Type::FixedArray.0 but used wrong in this case due https://github.com/gtk-rs/gir/blob/master/src/codegen/sys/ffi_type.rs#L94

@EPashkin
Copy link
Member

EPashkin commented Feb 6, 2018

Renaming also works before https://github.com/gtk-rs/gir/blob/master/src/codegen/sys/ffi_type.rs#L125,
but this will affect only sys'es

@sdroege
Copy link
Member Author

sdroege commented Feb 7, 2018

Generally, the type void alone in C is invalid, except for a return type or as a pointer. So renaming all void to gpointer seems an ok workaround, but quite ugly IMHO.

OTOH it seems impossible to correctly detect the right thing from the GIR?

@EPashkin
Copy link
Member

EPashkin commented Feb 7, 2018

@sdroege So decision up to you:

  1. you can fix GstVideo-1.0.gir (good that it only one with this error), with possibility report bug upstream,
  2. I can update Gir with fix in parser or in sys generation, this way we also silently fix some other crates,
    but upstream still be bad.

@sdroege
Copy link
Member Author

sdroege commented Feb 7, 2018

I'd vote for 1. (I can just fix it upstream anyway), and someone reports a bug against gobject-introspection?

@sdroege
Copy link
Member Author

sdroege commented Feb 7, 2018

@EPashkin
Copy link
Member

EPashkin commented Feb 7, 2018

Then I close this issue

@EPashkin EPashkin closed this as completed Feb 7, 2018
@sdroege
Copy link
Member Author

sdroege commented Feb 7, 2018

You report to gobject-introspection, or @tmiasko who debugged this in detail?

@EPashkin
Copy link
Member

EPashkin commented Feb 7, 2018

I had bad experience with gnome bugzilla, so I pass

@ghost
Copy link

ghost commented Feb 7, 2018

There is enough related bug reports in bugzilla that I don't feel that it is
necessary to report this one. See my comment referenced above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants