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

Fix out parameters as return for TypeKind other of Direct #117

Closed
9 tasks done
EPashkin opened this issue Sep 24, 2015 · 16 comments
Closed
9 tasks done

Fix out parameters as return for TypeKind other of Direct #117

EPashkin opened this issue Sep 24, 2015 · 16 comments

Comments

@EPashkin
Copy link
Member

Currently out parameters treated same way that result code like this

    fn query_child_packing<T: Upcast<Widget>>(&self, child: &T) -> (bool, bool, u32, PackType) {
        let mut expand.to_glib() = Default::default();
        let mut fill.to_glib() = Default::default();
        let mut padding = Default::default();
        let mut pack_type = Default::default();
        unsafe {
            ffi::gtk_box_query_child_packing(self.upcast().to_glib_none().0, child.upcast().to_glib_none().0, &mut expand.to_glib(), &mut fill.to_glib(), &mut padding, &mut pack_type);
        }
        (expand.to_glib(), fill.to_glib(), padding, pack_type)
    }

List of ffi functions which must return out parameters

  • gtk_alignment_get_padding
  • gtk_box_query_child_packing
  • gtk_button_get_alignment
  • gtk_calendar_get_date
  • gtk_frame_get_label_align
  • gtk_label_get_layout_offsets
  • gtk_layout_get_size
  • gtk_misc_get_alignment
  • gtk_misc_get_padding
homu added a commit that referenced this issue Sep 24, 2015
Limit out_as_return to unconverted types

First part of #117
homu added a commit that referenced this issue Sep 24, 2015
Comment functions with unsupported outs

Part of #117
@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

I think I might be fine with postponing all kinds of clever processing, but it's still difficult to settle on a uniform solution.

Simply using &mut T (Option<&mut T> for optional ones) doesn't work well for returning objects and boxed types (nothing to initialize the variable with at the call site). We could use &mut Option<T> and Option<&mut Option<T>> for that at a cost to ergonomics and wait for &out to become available. This is something that will work in all cases (we'll still have to add temporary variables when conversions are needed).

We could just put the actual return type and all the outparams in a tuple and return that but what about the optional outparams?

@EPashkin
Copy link
Member Author

It might be solution.
Only it is not clear whether or not to restrict the typekinds of return values.

@EPashkin
Copy link
Member Author

There functions with mixed outparams (optional and not)?
Any way to detect when parameters optional (besides #119) ?

@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

By optional I mean optional="1", you are allowed to pass NULL to ignore the value. Sometimes this has performance impact -- the function will take extra time to produce the value, but only if you actually want it. Can't find an example right now.

BTW optional seems to be the majority.

I don't know if there are combinations of optional and non-optional.

@EPashkin
Copy link
Member Author

In Gtk-3.0.gir no elements by XPath //parameter[@direction="out" and @optional="0"]
and many by //parameter[@direction="out" and not(@optional)]
Ex. gtk_accel_label_get_accel, gtk_box_query_child_packing, gtk_button_get_alignment.
Main problem IMHO how to determine whether the client needs these values, or we can not ask them.

@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

At this point I could even settle for treating all of them as non-optional, to be fixed/optimized later. This will allow returning a tuple; if the function isn't void, making the return value the first element.

@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

Need to look out for nullability. Much the same as with actual return values, they aren't tagged as nullable reliably.

@EPashkin
Copy link
Member Author

Agree.
As for not (bool and ()) returning functions, I found 7 (by XPath //method[return-value[type[@name!="gboolean" and @name !="none"]] and parameters/parameter[@direction="out"]]):
gtk_cell_area_get_cell_at_position -> CellRenderer,
gtk_icon_info_load_symbolic -> GdkPixbuf.Pixbuf,
gtk_icon_info_load_symbolic_finish -> GdkPixbuf.Pixbuf,
gtk_icon_info_load_symbolic_for_context -> GdkPixbuf.Pixbuf,
gtk_icon_info_load_symbolic_for_context_finish -> GdkPixbuf.Pixbuf,
gtk_icon_info_load_symbolic_for_style -> GdkPixbuf.Pixbuf,
gtk_tree_selection_get_selected_rows -> GLib.List

All this functions return objects, so it can be converted by XXX::From_glib_none.

@EPashkin
Copy link
Member Author

As result we have next "types" functions with out parameters by returning type:

  • () => (current mode) converted to (outs),
  • bool => converted to Option<(outs)>,
  • others => converted to (ret, outs)

@EPashkin
Copy link
Member Author

IMHO this is different issue (it main focus function return type, not types of outs). I try to add it tomorrow with list of functions, and then start to fix it.
Or we can use current №119 and add new for other return types.

@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

bool => converted to Option(outs),

I have a feeling this will be wrong for some functions.


We could turn this one into an outparams meta-issue.

@gkoz
Copy link
Member

gkoz commented Sep 24, 2015

() => (current mode) converted to (outs),
bool => converted to Option(outs),
others => converted to (ret, outs)

This looks good, we can weed out the exceptions later or just manually.

Oh, except if the function has an error outparam, in which case it's Result<(outs)>.

If the tuple has only one element it [the tuple] better be elided. ;)

@EPashkin
Copy link
Member Author

Returning errors can be mixed with out parameters (//method[@throws="1" and parameters/parameter[@direction="out"]] => 7 cases, //method[@throws="1" and not(parameters/parameter[@direction="out"])] => 42) and make 4th case which fully non covered.
For 7 cases:
gtk_builder_value_from_string and gtk_builder_value_from_string_type returns bool with out GValue,
5 returns GdkPixbuf.Pixbuf (#117 (comment)),
and last gtk_icon_info_load_symbolic_for_style also return GdkPixbuf.Pixbuf and deprecated in 3.0.
IMHO better first comment all functions with throw and on next stage focus on non out throws.

@EPashkin
Copy link
Member Author

As for elide tupple: it already done https://github.com/gkoz/gir/blob/master/src/codegen/function_body.rs#L80

@gkoz
Copy link
Member

gkoz commented Sep 25, 2015

Yes, throws is a separate topic but just in case

returns bool

If the function throws we should treat the bool return type as if it's void.

As for elide tupple: it already done

Cool :)

@EPashkin
Copy link
Member Author

@gkoz, I plan add new Issue about "Object returning functions with outs" same as I do in №130 and close this issue.

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