> 05:09 < owen> THe introspection information really should have the
> function as if it was "MyObject *my_object_new()"
> 05:09 < owen> Writing GtkWidget * is just a C convenience ... we know
> (statically - ahead of time) that isinstance(result, MyObject)

related to this, I have two methods in libsoup that take the wrong first argument type, for convenience:

    void soup_auth_domain_basic_set_auth_callback (SoupAuthDomain *domain,

it really ought to be a "SoupAuthDomainBasic *", but...

Anyway, there doesn't seem to be a way to annotate my way out of this currently. So a generic (actual-type TYPE) annotation might be good.


Another case that might make use of this is when you kludge a method's prototype to match a typedef:

    guint    soup_str_case_hash  (gconstpointer key);
    gboolean soup_str_case_equal (gconstpointer v1,
				  gconstpointer v2);

The gconstpointers here are all "really" const char *.

If I've understood it correctly, what tagging some static methods as constructors is trying to achieve is that given, for example, these "C constructors":
GtkWidget *gtk_button_new_with_label(const char *label);
GtkWidget *gtk_button_new_with_mnemonic(const char *mnemonic);

using 'is-constructor' flag on with_label() would mean that
1) the return value is really a) (transfer full) b) GtkButton and
2) if the language supports it, a 'GtkButton("foo")' constructor call should call with_label() rather than with_mnemonic()
3) no other method with similar signature can have 'is-constructor' flag

Now, 1a) is already default behavior for objects and has working annotation and 1b) is implicitly determined from the presence of '_new' in the function name, and should probably be supported through annotations as well (comment 1 and bug 561253)

2) feels like a potentially useful way to map constructor calls to function calls based on signature

3) what does "similar" mean, and who checks/enforces it? I'd guess the most universal rule would be to require constructors have different number of arguments to avoid possibilities for ambiguities due to different implicit type conversions in different languages.

There is a related problem to this. It doesn't only affect static methods, but also constructors which take an object of the same class as first argument. I just stumbled over this while working on bug 635253. Gtk has a radio button constructor:

/**
 * gtk_radio_button_new_with_label_from_widget:
 * @radio_group_member: (allow-none): widget to get radio group from or %NULL
 * @label: a text string to display next to the radio button.
 *
 * Creates a new #GtkRadioButton with a text label, adding it to
 * the same group as @radio_group_member.
 *
 * Returns: (transfer none): a new radio button.
 */
GtkWidget*
gtk_radio_button_new_with_label_from_widget (GtkRadioButton *radio_group_member,
                                             const gchar    *label)
{

But g-i doesn't consider that a constructor even though it has "_new_" in it, presumably because it also looks at the type of the first argument and misinterprets that as "self", and thus considers this a method?

This gets turned into
      <method name="new_with_label_from_widget"
              c:identifier="gtk_radio_button_new_with_label_from_widget">
        <doc xml:whitespace="preserve">Creates a new #GtkRadioButton with a text label, adding it to
the same group as @radio_group_member.</doc>
        <return-value transfer-ownership="none">
          <doc xml:whitespace="preserve">a new radio button.</doc>
          <type name="Widget" c:type="GtkWidget*"/>
        </return-value>
        <parameters>
          <parameter name="label" transfer-ownership="none">
            <doc xml:whitespace="preserve">a text string to display next to the radio button.</doc>
            <type name="utf8" c:type="gchar*"/>
          </parameter>
        </parameters>
      </method>

which both drops the constructor behaviour, and also silently drops the "allow-none", since the first parameter disappeared entirely.

So I agree that there should be a way to force a method to be a constructor for corner cases like this.

Thanks!

Do we know how common is the problem that Martin mentions?

If it's very uncommon and just happens with convenience constructors, we could mark those as introspectable=0 and let bindings authors add overrides if they really think it brings good valued to application authors.

This may be more widespread than I thought, we also have this problem in

GdkWindow* gdk_window_new (GdkWindow *parent, GdkWindowAttr *attributes, gint attributes_mask)

This should be either a constructor or a (factory) static method, but not a method.

Created attachment 178631 [details] [review]
Allow constructors with instances of the same class as their first argument

So we can use constructors such as gtk_radio_button_new_with_label_from_widget
and gdk_window_new.

(In reply to comment #6)
> Created an attachment (id=178631) [details] [review]
> Allow constructors with instances of the same class as their first argument
> 
> So we can use constructors such as gtk_radio_button_new_with_label_from_widget
> and gdk_window_new.

Tests in g-i pass without modification and by diffing the gtk and gdk .gir we can see that this causes only the intended changes.

Review of attachment 178631 [details] [review]:

There was *definitely* a reason this code was added; if I use "git annotate", I get:

commit 5140ec172415192af4f406b1fd5ff4615caf1386
Author: Colin Walters <walters@verbum.org>
Date:   Wed Sep 8 12:18:51 2010 -0400

    scanner: Don't mark as constructors things that are more obviously methods
    
    If the first parameter matches the origin, don't scan as a constructor.
    Happened in practice with meta_screen_append_new_workspace from mutter.

So, here we have a method that just happens to have "_new" in the name.  I don't see a way to have both:

* The heuristic on _new
* Somehow picking between method and constructor automatically

We've obviously needed annotations for a long time here...but I'm not sure which way to lean.  I think I'd say we should require annotations on constructors that take an instance as the first argument.

Created attachment 178952 [details] [review]
Add (constructor) annotation

One more possibility to deal with this issue

Review of attachment 178952 [details] [review]:

::: giscanner/maintransformer.py
@@ +576,3 @@
             node.foreign = True
 
+        if OPT_CONSTRUCTOR in block.options and hasattr(node, 'is_constructor'):

isinstance(node, Function) instead of hasattr() please.

@@ +964,2 @@
     def _pair_constructor(self, func, subsymbol):
+        if not func.is_constructor and \

This is similar to the one I commented on in the (function) annotation in that it seems weird to me to mix the case where the user explicitly told us something is a constructor via annotation with the case where we're trying to infer it.

If you could do the same refactoring here I think the code would come out significantly cleaner.

Or at least, it really deserves a comment.  Right now the code will be too mysterious for someone reading it later.

Created attachment 179507 [details] [review]
Add (constructor) annotation

Review comments addressed.

Review of attachment 179507 [details] [review]:

Looks good, thanks.

Thanks!

Attachment 179507 [details] pushed as 2c36790 - Add (constructor) annotation

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]