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 774713 - Generics are broken with non-pointer types
Generics are broken with non-pointer types
Status: RESOLVED OBSOLETE
Product: vala
Classification: Core
Component: Generics
0.34.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
: 647618 648366 672099 725759 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-11-19 13:15 UTC by Colomban Wendling
Modified: 2018-05-22 15:41 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Colomban Wendling 2016-11-19 13:15:30 UTC
Generic support for fundamental value types like int, long, double, float, or even enum and flags is currently broken when `sizeof(gpointer) != sizeof(G)`.

The problem is easy to understand: Vala uses `gpointer` as the C type for generics, but fundamental value types do not necessarily work with that.
There are good examples of the problem e.g. in libgee's https://bugzilla.gnome.org/show_bug.cgi?id=597737 and https://bugzilla.gnome.org/show_bug.cgi?id=774669.

Even simple code like this will have problem if `sizeof(G) > sizeof(gpointer)`, as the value will be truncated:

    G value_generic<G>(G x) {
      return x;
    }

calling `value_generic<int64>(4200000000000000000)` on a 32 bits architecture will not result in the expected `4200000000000000000`.  FWIW, with GCC's -m32 on a Linux x86_64, it gives me `1487142912`.

However, this part isn't *too* problematic, as it only applies for types larger than `gpointer`, and, on x86, only when the value actually is larger: `value_generic<int64>(42)` actually works as the truncation doesn't lose anything relevant.

But it gets real problematic when doing more subtle things, like libgee's Collection.to_array(): creating an array of Gs:

    G[] array_generic<G>(G x) {
      G[] y = new G[2];
      y[0] = x;
      y[1] = x;
      return y;
    }

The problem is that it is expected that using array_generic<int>() would be the same as replacing every occurrence of `G` in the definition with `int`.  But it is not, it actually is `gpointer`, so the function creates an array of `gpointer`s, which on an x86_64 Linux doesn't have the same layout as an array of `int`s (`int`s being 4 bytes, and pointers 8).  So the array returned by `array_generic<int>()` *cannot* be used as an `int[]`.  Worse, if the generic type is larger than `gpointer`, it'll result in a too small array the caller will happily access outside its actual bounds.

libgee worked around *some* of the issue in its `Collection.to_array()` implementation by special-casing most fundamental value types (`char`, `uchar`, `int`, `uint`, etc.), and providing an implementation actually working on those specific types -- see https://git.gnome.org/browse/libgee/tree/gee/collection.vala#n158  However, they missed enumerations and flags -- see https://bugzilla.gnome.org/show_bug.cgi?id=774669 -- which are actually trickier to support (see below).


I unfortunately don't see a proper solution for the issue, at least not one that would be mostly compatible and fix everything.  Maybe using `GValues` for passing the generics by value would slightly help.  Or always use the largest possible type for them instead of `gpointer`?

For the array part, it could probably be hacked around manually playing with the memory: all it really requires is knowing the size of the type, and probably the layout of the actually used type in memory (`gpointer` currently).  Then, generics could use convoluted logic like that:

    gpointer* array_generic (GType T, ..., gpointer x) {
      guint8* result = g_malloc0_n (n, size_of_T);
      memcpy(result + size_of_T*0, (guint8*) x, size_of_T);
      memcpy(result + size_of_T*1, (guint8*) x, size_of_T);
      return (gpointer*) result;
    }

so, so long as the caller casts it back to the real type, it would work -- unless `size_of_T` is larger than `gpointer`, again.  Reading `x` might require some slightly more convoluted code though if the relevant part of the value isn't packed at the start of the `gpointer`, but that would be doable too knowing enough about the platform I guess.

Another possible simpler/safer solution would be doing the same as libgee's in the compiler itself: emit specialized versions to be used for basic types.  It would result in larger generated code, but it might be worth it.  But it still doesn't really work good when `sizeof(G) > sizeof(gpointer)` as some truncation will occur passing some data as `gpointer`s (here, the `x` argument).


Supporting enumerations and flags is even trickier, because their actual type size is trickier to know.  GCC will use `int` when it can, but will happily switch to a larger type if the enumeration has values larger than what an `int` can represent.  And this means we need to know the *actual* size of the enumeration to properly support that.  Would it be a property on GEnumClass it would be easy.  But unfortunately it isn't, and worse, GEnumClass doesn't actually really support enums larger than `int` (as minimum and maximum are `int`s, it's impossible to even guess the required size).
So… maybe it's good enough to support them only when their size `== sizeof(int)`?  Not sure, but at least it would work in *most* cases.


In the end, I guess the most realistic short-term solution would be using something like libgee's approach.  But I have no good idea for a perfect long-term solution.
Comment 1 Al Thomas 2016-11-19 19:24:39 UTC
What's your analysis of "boxing"? e.g. 
value_generic<int64?>(4200000000000000000);

If it offers any insight, klib take a different approach to generics:
https://github.com/attractivechaos/klib
https://attractivechaos.wordpress.com/2008/10/02/using-void-in-generic-c-programming-may-be-inefficient/
Comment 2 Colomban Wendling 2016-11-19 21:36:26 UTC
(In reply to Al Thomas from comment #1)
> What's your analysis of "boxing"? e.g. 
> value_generic<int64?>(4200000000000000000);

Well, I guess it's just fine.  It has the downside of allocating memory for it (as Vala will happily dup it often), but it's a valid solution.

However looking at the generated C code, it makes me afraid that Gee.Collection.to_array() is totally broken with those maybe-types: it uses typeof(G) to find out the type, but it has the same result on maybe-types and not -- that is, `if (typeof(G) == typeof(int64))` will be true both for `int64` and `int64?`, yet the actual C type is different.

And this doesn't really solve the array example, as it will only actually make it return an array of pointers to int64 values, and not an array of int64 values.

In any case, the most important point IMO would be to make sure Vala doesn't happily generate incorrect code.  If non-maybe non-pointer types are not properly supported, the compiler should say so.

> If it offers any insight, klib take a different approach to generics:
> https://github.com/attractivechaos/klib
> https://attractivechaos.wordpress.com/2008/10/02/using-void-in-generic-c-
> programming-may-be-inefficient/

Well, the idea is mostly the same as libgee's: use specialized implementations.  But indeed it can be pretty interesting: Vala could emit and choose specialized versions of generics for the types which actually use them (so `value_generic<int64>()` would expand to something like `value_generic_int64()`, a specialized version emitted for the occasion).  But that only work if you can emit this code at compile time: if the generic is part of a library API, either the using program will have to know how to emit the specialized code, or the selection has to be dynamic (as libgee is doing), and then there still need to be a generic C type.

BTW, generating specialized versions for every possible non-pointer types might be a little costly, especially with multiple generics: it's N**M, where N is the number of types for which there is a specialized version, and M the number of generics that function takes.  Which is already 144 for 2 generics if we consider there are to be 12 specialized variants: bool, char, uchar, int, uint, long, ulong, int64, uint64, float, double, and pointer.  It might be OK not to have specialized versions for signed vs. unsigned however, which would bring that 12 down to a 8 -- and so down to 64 for 2 generics.

So I guess whether mentioning klibc's approach is relevant depends on whether Vala's generic have to be dynamic, or can be static: that is, whether selecting the specialization is something that can be done at compile time, or if it has to be doable at runtime.  Though, there could possibly be an hybrid solution where the compiler generates a dynamic version, but calls in the specialized version directly when it can -- and so make the decision at compile time.
Comment 3 Al Thomas 2016-11-30 14:03:28 UTC
*** Bug 648366 has been marked as a duplicate of this bug. ***
Comment 4 Al Thomas 2016-11-30 14:29:37 UTC
*** Bug 647618 has been marked as a duplicate of this bug. ***
Comment 5 Al Thomas 2016-11-30 17:58:00 UTC
*** Bug 672099 has been marked as a duplicate of this bug. ***
Comment 6 Al Thomas 2018-02-24 15:35:09 UTC
*** Bug 725759 has been marked as a duplicate of this bug. ***
Comment 7 GNOME Infrastructure Team 2018-05-22 15:41:27 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/vala/issues/564.