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

Take argument type for GTypeInstance argument from argument value on SCM procedure invocation #104

Closed
Bob131 opened this issue Jan 21, 2021 · 7 comments

Comments

@Bob131
Copy link

Bob131 commented Jan 21, 2021

The following situation cropped up writing some async code: A procedure is passed to g_task_new() as the callback. When the task is marked as returned the callback is invoked with the GTask instance as an argument; however, guile-gi produces a GOOPS instance of type <GAsyncResult> (per the GI metadata) which cannot be used with any of the <GTask> methods.

For reference, PyGObject instead constructs a new PyObject using the argument's GType (see here)

@spk121
Copy link
Owner

spk121 commented Jan 24, 2021

OK. If I understand this correctly, in C normally one would just cast an GAsyncResult to a GTask with
G_TASK like in this code snippet

    baker_bake_cake_finish (Baker         *self,
                            GAsyncResult  *result,
                            GError       **error)
    {
      g_return_val_if_fail (g_task_is_valid (result, self), NULL);

      return g_task_propagate_pointer (G_TASK (result), error);
    }

So the problem here is the lack of an introspected method to cast <GAsyncResult> to <GTask>,
which is why you'd like guile-gi to provide a workaround. Is that right?

@Bob131
Copy link
Author

Bob131 commented Jan 24, 2021

OK. If I understand this correctly, in C normally one would just cast an GAsyncResult to a GTask with
G_TASK like in this code snippet

    baker_bake_cake_finish (Baker         *self,
                            GAsyncResult  *result,
                            GError       **error)
    {
      g_return_val_if_fail (g_task_is_valid (result, self), NULL);

      return g_task_propagate_pointer (G_TASK (result), error);
    }

Yep.

So the problem here is the lack of an introspected method to cast <GAsyncResult> to <GTask>,
which is why you'd like guile-gi to provide a workaround. Is that right?

Well, that's not wrong, but I wouldn't call it a 'workaround'. Rather, I'd argue that having to use G_TASK is a workaround for static typing, and that the right thing to do in a binding for a dynamically-typed language would be to always represent values in a manner consistent with their true dynamic type (hence the PyGObject behaviour).

spk121 added a commit that referenced this issue Jan 24, 2021
…sion

* src/gig_argument.c (c_interface_to_scm, c_object_to_scm):
    query type from pointer
@spk121
Copy link
Owner

spk121 commented Jan 24, 2021

I think that commit fixes. I'll close after I write appropriate tests.

@Bob131
Copy link
Author

Bob131 commented Jan 24, 2021

Thanks!

I might also point out that GObject is not the only classed type possible with the GType system (which is why I mentioned GTypeInstance in the title), but AFAIK not treating them specially here wouldn't be out of step with other language bindings (to rephrase without the double negative: the proposed commit is perfectly adequate :)

spk121 added a commit that referenced this issue Jan 26, 2021
* Makefile.am: add test/task.scm
* test/task.scm: new <GTask> tests
spk121 added a commit that referenced this issue Jan 26, 2021
* src/gig_argument (child_type): new helper
  (c_interface_to_scm, c_object_to_scm, c_variant_to_scm)
  (c_param_to_scm): prefer self-reported types
spk121 added a commit that referenced this issue Jan 26, 2021
* src/gig_argument (child_type): new helper
  (c_interface_to_scm, c_object_to_scm): prefer self-reported types
@spk121
Copy link
Owner

spk121 commented Jan 26, 2021

@Bob131 implied that there are other types beyond objects/interfaces that should be handled, but, looking at how arguments get dragged through the two-step FFI->C->SCM and back again, I think only GObjects/GInterfaces can carry their own type information through that process, because the type lives within their C pointer.

GBoxed could be a possibility here, where a GBoxed pointer becomes a naked C pointer through the SCM->C->FFI->C->SCM
But only if the pointer is both in and out.

I suppose one could do a bit of a hack in function_binding to make a GType->pointer hash of any GBoxed arguments before the function call, and then examine any GBoxed pointers returned from the function call to see if they are in the hash, to restore their original GType. That wouldn't work in callback_binding, obviously.

But is this a real or theoretical issue? I don't know.

@Bob131
Copy link
Author

Bob131 commented Jan 26, 2021

Uh, well, there are varying degrees of 'real' ;)

All GTypeInstances carry a reference to their GTypeClass, which itself stores the GType (hence GObject's behaviour). IIRC GStreamer used to have issues before GstMiniObject et al were boxed (but don't quote me on that). GParamSpec is another instance of a non-GObject GTypeClass hierarchy.

They're generally a pain in C and not really supported by most language bindings (in my experience, anyway), but Vala makes it dead-easy to create new GTypeClass hierarchies which can be handy for code requiring instantiation of lots of instances (where GObject might otherwise be too heavyweight but you still want runtime type reflection or GInterface or something). That said, I haven't done anything like that for a while, so it's not really something with which I'm concerned.

It's probably plenty safe to exclude support for it, I only really mention it because it used to be a pain-point and people usually were surprised to learn about said functionality.

@spk121
Copy link
Owner

spk121 commented Jan 26, 2021

@ebassi from GTK says the future of GTK should just be GObject or GBoxed, and that if it is GBoxed, it should have no class hierarchy, in his blog article on type instances . So I'll take that as evidence that my laziness here is justified, and close the issue.

@spk121 spk121 closed this as completed Jan 26, 2021
@spk121 spk121 added this to the v0.3.2 milestone Jan 27, 2021
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