After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 558620 - Add default values
Add default values
Status: RESOLVED OBSOLETE
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Guillaume Emont (guijemont)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 559704 640812
 
 
Reported: 2008-10-31 01:30 UTC by Johan (not receiving bugmail) Dahlin
Modified: 2018-02-08 11:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v1 (10.17 KB, patch)
2008-10-31 01:31 UTC, Johan (not receiving bugmail) Dahlin
none Details | Review
scanner: Add support for the default annotation (6.80 KB, patch)
2011-08-12 15:04 UTC, Guillaume Emont (guijemont)
needs-work Details | Review

Description Johan (not receiving bugmail) Dahlin 2008-10-31 01:30:21 UTC
We should add support for default values. It makes sense to use them in most languages.

The following needs to be done:
- Annotation syntax: (default value)
- Gir markup default="value"
- Typelib implementation
- Typelib specification

Plus a bunch of tests for all of it
Comment 1 Johan (not receiving bugmail) Dahlin 2008-10-31 01:31:35 UTC
Created attachment 121705 [details] [review]
v1

Initial take: compiles but misses a little bit of love in the typelib part. Failing in strange ways when compiling the GLib typelib.
Comment 2 Tommi Komulainen 2008-10-31 10:40:21 UTC
Considering that typelib is platform dependent, and the value type is already known, any reason to not write the native value (or offset to string) directly in the typelib and do parsing only at compile time?


Either way, IMO a better API would be something more like:

gboolean
g_arg_info_default_value(GIArgInfo *info, GArgument *arg)
{
  if (!blob->default_value)
    return FALSE;

  if (type is string)
    arg->v_pointer = g_typelib_get_string (..);
  else if (type is double)
    arg->v_double = ..;
  ...
  return TRUE;
}


I think magic '"NULL" (string) means NULL (pointer)' in band values should be avoided, and something like default-null="1" would be better. But maybe that's just me.
Comment 3 Guillaume Emont (guijemont) 2011-08-12 15:04:03 UTC
Created attachment 193705 [details] [review]
scanner: Add support for the default annotation

This is the annotation scanner/gir generation part.
Comment 4 Guillaume Emont (guijemont) 2011-08-12 15:09:52 UTC
There is still one question remaining for the scanner: should we allow default values for a parameter when there is another parameter after it that does not have a default value? Are there languages in which that would make sense? For sure that is forbidden in python.
Comment 5 Colin Walters 2011-08-13 15:34:42 UTC
So my thought on adding this to the typelib is that we:

* Take one of the reserved bits in GIArgInfo
* If set, that means we look into a new section in the typelib (like the directory index section which was added in commit 24a2b4c1543798fe61fd6e3667b12e44766740ae)
* The new section has a mapping from C symbol,param offset to a ConstantBlob

To make this efficient we may need to do something like what's done for the directory index section and make a hash table out of it.
Comment 6 Guillaume Emont (guijemont) 2011-08-14 10:04:15 UTC
An alternative of it would be:

 * Take one of the reserved bits in GIArgInfo
 * If set, that means we look into an array of "ValueBlobs" appended to SignatureBlob
 * For size efficiency, we have a SignatureBlob appended iff the corresponding ArgInfo has a "has_default_value" bit set, a separate value blob index/counter needs to be maintained to know where to look in the ValueBlob array.

ValueBlob would be something simple like:
typedef struct {
  guint32 size;
  guint32 offset;
} ValueBlob;

I don't think we need more info because we already have the type info in the ArgBlob.

Pros: the lookup would likely be faster as we don't need to calculate any hash
Cons:
 - it might be a bit inconsistent to store argument-related info outside of ArgBlob
 - need to change the size of SignatureBlob and the ValueBlob array won't be in a constant offset in SignatureBlob; I'm not sure whether this is a source of problem or not
Comment 7 Colin Walters 2011-08-14 10:14:22 UTC
Hmm.  It looks to me actually like ConstantBlob as is now is broken for non-string values?  

girnode.c:815
	/* FIXME non-string values */
	size += ALIGN_VALUE (strlen (constant->value) + 1, 4);

The size/offset pairing to me looks weird to me.  In g_constant_info_get_value() we do:

      if (blob->type.flags.pointer)
	value->v_pointer = g_memdup (&rinfo->typelib->data[blob->offset], blob->size);

But we could just do g_strdup() assuming we only support UTF-8 constants.

Oh...it looks like we're storing 32 bit constants in the offset directly, and we're broken for 64 bit values (guint64, double).

See also https://bugzilla.gnome.org/show_bug.cgi?id=654069

So we may need to drain that swamp first.

I think instead of a pair of size/offset, what we want is just a guint64.  If the type is a string/filename, then it's an offset.  Otherwise we read the value directly.
Comment 8 Colin Walters 2011-08-14 10:18:09 UTC
Leaving aside the ConstantBlob brokenness, I think your plan with ValueBlob is fine, but it'd be:

ValueBlob {
  guint16 tag;
  guint64 value
};

or something like that.  I'm pretty sure we can expand SignatureBlob; it's not part of any array.  Having it be a dynamic offset is OK.
Comment 9 Guillaume Emont (guijemont) 2011-08-14 12:57:16 UTC
Do we really need the tag part? Type info is already available from the ArgBlob (in .arg_type).
I think I will use a simple guint64 for now.
Comment 10 Evan Nemerson 2013-03-29 21:26:58 UTC
It seems like the hold up here is the typelib bits... would it be possible to get the GIR part pushed by itself?  It would be great to be able to make use of this in Vala.
Comment 11 Mikhail Zabaluev 2014-10-04 17:48:15 UTC
This would be useful for g_locale_from_utf8().
A binding would convert its representation of a Unicode string for @utf8string (correctly auto-annotated as type utf8), but its clients shouldn't be forced to supply the correct value for @len.
Comment 12 André Klapper 2015-02-07 17:14:09 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]
Comment 13 Richard Hughes 2015-07-31 11:22:08 UTC
I have need of this myself in AppStreamGlib, although this bug looks pretty old and forgotten about :(
Comment 14 Alberto Ruiz 2017-03-30 16:19:25 UTC
I have need of this for Rust bindings
Comment 15 Emmanuele Bassi (:ebassi) 2017-03-30 17:09:43 UTC
(In reply to Alberto Ruiz from comment #14)
> I have need of this for Rust bindings

A good start would be:

 - rebase attachment 193705 [details] [review] onto current master
 - split the GIR and typelib bits into two separate commits

Both Vala and Rust would consume the GIR anyway, AFAIR, so they would immediately get the benefit of a default value; it would also help documentation generators like HotDoc that use GIR as a source.

Then we can look at how to implement this in the typelib for dynamic languages.
Comment 16 Emmanuele Bassi (:ebassi) 2017-03-30 17:10:39 UTC
De-ASSIGN this bug, since it hasn't seen any activity since 2011.
Comment 17 GNOME Infrastructure Team 2018-02-08 11:45:30 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/4.