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

Fixed deriving Copy trait for struct with function with varargs #719

Merged
merged 2 commits into from Feb 9, 2019

Conversation

Projects
None yet
3 participants
@EPashkin
Copy link
Member

EPashkin commented Feb 8, 2019

Fix #716

Now gir understand that this field - function is incomplete

Option<unsafe extern "C" fn(*mut DzlPreferences, *const c_char, *const c_char,
  *mut gtk::GtkWidget, /*Unimplemented*/va_list) -> c_uint>

Has no effect on gtk-rs

cc @GuillaumeGomez, @sdroege, @BrainBlasted

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 8, 2019

What does it generate now? The va_list part should simply be a ... like here

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 8, 2019

It's impossible to define such functions in Rust right now, but you can at least name them and call them :)

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Feb 8, 2019

@sdroege This slightly different types (at minimum now)
Your gst_caps_new_full has parameter with gir's type Fundamental::VarArgs

          <parameter name="..." transfer-ownership="none">
            <doc xml:space="preserve">additional structures to add</doc>
            <varargs/>
          </parameter>

Field add_table_row_va of record DzlPreferencesInterface is callback with

            <parameter name="args" transfer-ownership="none">
              <type name="va_list" c:type="va_list"/>
            </parameter>
@@ -13,12 +13,12 @@ pub trait IsPtr {
impl IsPtr for Field {
fn is_ptr(&self) -> bool {
if let Some(ref c_type) = self.c_type {
return c_type.contains('*')
return c_type.contains('*');

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Feb 8, 2019

Member

No need for return in here.

This comment has been minimized.

@EPashkin

EPashkin Feb 9, 2019

Author Member

Thanks, I don't notice that many of unneeded early returns here

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Feb 8, 2019

Besides the nit, all good for me! :)

@EPashkin EPashkin force-pushed the EPashkin:incomplete branch from e6ce658 to 8a9024d Feb 9, 2019

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Feb 9, 2019

@sdroege If you think that varags and va_list must generated same code, then I add other PR

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 9, 2019

@sdroege If you think that varags and va_list must generated same code, then I add other PR

That's exactly what va_list is, yes. They should be treated the same and it's not clear why gobject-introspection does not do that :)

For the record, rust-lang/rust#44930 is the RFC for being able to define such varargs functions from Rust.

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Feb 9, 2019

Seems is not same as this is 2 different function,

int vprintf ( const char * format, va_list arg );
int printf ( const char * format, ... );
@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 9, 2019

Oh indeed, I misread. Sorry!
... is the "unexpanded" form of va_list. So yes, these are different. What we have here is basically this

@EPashkin

This comment has been minimized.

Copy link
Member Author

EPashkin commented Feb 9, 2019

Then I merge this.

@EPashkin EPashkin merged commit 2723216 into gtk-rs:master Feb 9, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@EPashkin EPashkin deleted the EPashkin:incomplete branch Feb 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.