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 784691 - array_length_cexpr does not have an effect on return values
array_length_cexpr does not have an effect on return values
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-08 03:26 UTC by David Lechner
Modified: 2017-09-21 12:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to fix bug (1.06 KB, patch)
2017-07-08 04:21 UTC, David Lechner
none Details | Review
codegen: Use array_length_cexpr to support fixed-arrays for return-values (4.09 KB, patch)
2017-07-08 11:26 UTC, Rico Tzschichholz
committed Details | Review

Description David Lechner 2017-07-08 03:26:54 UTC
I'm trying to bind a function that returns a fixed size array.

C definition:

/**
 * grx_color_alloc_ega_colors:
 *
 * Allocates an array of the 16 classic EGA colors. Use #GrxEgaColorIndex to
 * determine the color at each index of the array.
 *
 * Returns: (array fixed-size=16): array of color indexes
 */
GrxColor *grx_color_alloc_ega_colors(void);


Vapi:

        [CCode (array_length_cexpr = "16")]
        public static unowned Color[] alloc_ega_colors ();

However, array_length_cexpr is ignored and the generated code is the same as if the CCode attribute was omitted altogether. In other words...

    _tmp1_ = grx_color_alloc_ega_colors (&_tmp0_);


I also tried:

        [CCode (array_length = false, array_length_cexpr = "16")]
        public static unowned Color[] alloc_ega_colors ();

But that results in length = -1:

	_tmp0_ = grx_color_alloc_ega_colors ();
	self->priv->colors = _tmp0_;
	self->priv->colors_length1 = -1;


I was able to work around with:

        [CCode (cname = "grx_color_alloc_ega_colors", array_length = false)]
        static unowned Color[] _alloc_ega_colors ();
        [CCode (cname = "vala_grx_color_alloc_ega_colors")]
        public static unowned Color[] alloc_ega_colors () {
            unowned Color[] colors = _alloc_ega_colors ();
            colors.length = 16;
            return colors;
        }
Comment 1 David Lechner 2017-07-08 04:21:46 UTC
Created attachment 355139 [details] [review]
Patch to fix bug

Tested that it fixes the problem and that it does not break current behavior when CCode attribute is committed.
Comment 2 Rico Tzschichholz 2017-07-08 11:26:14 UTC
Created attachment 355169 [details] [review]
codegen: Use array_length_cexpr to support fixed-arrays for return-values

Based on patch by David Lechner
Comment 3 Rico Tzschichholz 2017-07-08 11:32:16 UTC
I have extended your patch a bit by including girparser-support and adding according test-cases.
Comment 4 David Lechner 2017-07-08 15:06:23 UTC
Review of attachment 355169 [details] [review]:

::: tests/gir/array-fixed-length.test
@@ +63,3 @@
 [CCode (cheader_filename = "test.h")]
 public static void get_array_out ([CCode (array_length = false)] out uint8 array[8]);
+[CCode (array_length = false, array_length_cexpr = "16", cheader_filename = "test.h")]

I don't think it makes sense to have both array_length = ... and array_length_cexpr = ...

Shouldn't array_length_cexpr =... imply that array_length = false?

If not, then the code generator needs another look.
Comment 5 Rico Tzschichholz 2017-07-08 19:13:03 UTC
Annotating with "array_length = false" seems redundant and of course it is correct from the ccode perspective. Might be better to keep the FIXME since this is still quite a hack.

Native support of fixed-length array as return-value, e.g. "int[16] get_foo () { ... }" would be the proper solution to avoid ccode-attributes.
Comment 6 Rico Tzschichholz 2017-09-21 12:44:06 UTC
Attachment 355169 [details] pushed as a948a3f - codegen: Use array_length_cexpr to support fixed-arrays for return-values