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 640330 - Vala produces non compiling code when virtual function requires generic argument from interface
Vala produces non compiling code when virtual function requires generic argum...
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
0.14.x
Other Linux
: Normal normal
: ---
Assigned To: Jeremy Whiting
Vala maintainers
invalid-c-code
: 632410 (view as bug list)
Depends on:
Blocks: 589968 661102 679587
 
 
Reported: 2011-01-23 18:17 UTC by Maciej (Matthew) Piechotka
Modified: 2012-08-06 10:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The first of three changes needed to fix the bug. (1.19 KB, patch)
2012-07-11 18:00 UTC, Jeremy Whiting
none Details | Review
Updated patch to lower case function names and connect in interface_init method (4.21 KB, patch)
2012-07-11 19:07 UTC, Jeremy Whiting
none Details | Review
Updated to not use Method objects (6.87 KB, patch)
2012-07-18 17:11 UTC, Jeremy Whiting
none Details | Review
Adds get_x_type, get_x_dup_func, and get_x_destroy_func to interfaces with GenericAccessors attribute. (10.33 KB, patch)
2012-07-18 21:16 UTC, Jeremy Whiting
none Details | Review
Updated patch to implement get_x_type get_x_dup_func and get_x_destroy_func in implementation classes (11.28 KB, patch)
2012-07-18 22:50 UTC, Jeremy Whiting
none Details | Review
Updated patch to connect interface to implementation and fix the type passed in for self (11.80 KB, patch)
2012-07-19 17:35 UTC, Jeremy Whiting
none Details | Review
Updated last patch to add casting in interface_init connection code. (11.91 KB, patch)
2012-07-19 18:34 UTC, Jeremy Whiting
none Details | Review
Updated patch (8.89 KB, patch)
2012-07-28 16:56 UTC, Jürg Billeter
none Details | Review
Updated patch (9.36 KB, patch)
2012-07-30 15:00 UTC, Jürg Billeter
none Details | Review

Description Maciej (Matthew) Piechotka 2011-01-23 18:17:15 UTC
public virtual Iterator<B> map<B> (owned MapFunc<G, B> map) {
		return new MappingIterator<G, B> (this, (owned) map);
	}

  CC     libgee_la-iterator.lo
iterator.c: In function ‘gee_iterator_map’:
iterator.c:237:41: error: dereferencing pointer to incomplete type
iterator.c:237:78: error: dereferencing pointer to incomplete type


        _tmp3_ = gee_mapping_iterator_new (self->priv->g_type, (GBoxedCopyFunc) self->priv->g_dup_func, NULL, b_type, (GBoxedCopyFunc) b_dup_func, NULL, self, _tmp0_, _tmp1_, _tmp2_);
Comment 1 Maciej (Matthew) Piechotka 2011-03-27 09:15:31 UTC
*** Bug 632410 has been marked as a duplicate of this bug. ***
Comment 2 Maciej (Matthew) Piechotka 2011-10-02 13:08:35 UTC
Possible solution:

 - Abstract property overloaded by child class
 - Abstract function overloaded by child class

Ex.

[ConcretGenerics ()]
public interface Test<G> {
    ....
}

public class TestTest<G> : Test<G> {
  // Automatically override the properties
}
Comment 3 Jeremy Whiting 2012-07-11 18:00:20 UTC
Created attachment 218580 [details] [review]
The first of three changes needed to fix the bug.

After discussing with Juerg we decided the best idea was to use a function in the generated c code to get the types for the generics used at run time.  This patch adds virtual abstract functions for each generic type.
Comment 4 Jeremy Whiting 2012-07-11 19:07:46 UTC
Created attachment 218585 [details] [review]
Updated patch to lower case function names and connect in interface_init method
Comment 5 Jeremy Whiting 2012-07-18 17:11:36 UTC
Created attachment 219144 [details] [review]
Updated to not use Method objects

Thanks to a lot of guidance from Juerg and Lethalman, this patch now generates interface_get_x_type functions, however it's still not complete.  I'm currently getting an error in the c code where I have

return typeof (G);

it complains that "unexpected expression before 'typeof'"
Comment 6 Jeremy Whiting 2012-07-18 21:16:38 UTC
Created attachment 219158 [details] [review]
Adds get_x_type, get_x_dup_func, and get_x_destroy_func to interfaces with GenericAccessors attribute.

This patch makes vala create buildable code for the libgee branch that triggered this bug.  It doesn't however solve the bug completely, still need to add code to generate the methods in the classes that implement the interface.
Comment 7 Jeremy Whiting 2012-07-18 22:50:48 UTC
Created attachment 219172 [details] [review]
Updated patch to implement get_x_type get_x_dup_func and get_x_destroy_func in implementation classes

This generates code by calling get_type_id_expression, get_dup_func_expression, and get_destroy_func_expression.  However the code this generates currently does not build for libgee :(

It's generating stuff like the following for a class that implements Gee Traversable

GType traversable_get_g_type (GeeTraversable* self) {
        return self->priv->g_type;
}

but it should probably be wrapping self in GEE_ABSTRACT_COLLECTION() which is the implementation class.
Comment 8 Jeremy Whiting 2012-07-19 17:35:28 UTC
Created attachment 219250 [details] [review]
Updated patch to connect interface to implementation and fix the type passed in for self

Now everything builds and runs ok from what I have seen.

One minor thing missing is the iface->get_x_type = class_get_x_type; doesn't cast the function pointer, but that only generates a warning so wasn't added yet.
Comment 9 Jeremy Whiting 2012-07-19 18:34:22 UTC
Created attachment 219256 [details] [review]
Updated last patch to add casting in interface_init connection code.

I believe this is the last update needed to this patch to fix this bug once and for all.  Any code review is welcome, at this point I recommend the patch be applied to master, but I'm biased as I wrote it.  Let me know if I need to tweak anything else for it to be acceptable though.
Comment 10 Jürg Billeter 2012-07-28 16:56:19 UTC
Created attachment 219795 [details] [review]
Updated patch

I've simplified the patch a bit and fixed up the coding style. Can you please check whether this is still ok from your side?
Comment 11 Maciej (Matthew) Piechotka 2012-07-30 00:03:38 UTC
(In reply to comment #10)
> Created an attachment (id=219795) [details] [review]
> Updated patch
> 
> I've simplified the patch a bit and fixed up the coding style. Can you please
> check whether this is still ok from your side?

I've checked the new patch. It looks like at least the new one (I haven't check the older one in this respect) does not compile the master branch of libgee even though the Vala master works fine.
Comment 12 Jürg Billeter 2012-07-30 14:56:08 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Created an attachment (id=219795) [details] [review] [details] [review]
> > Updated patch
> > 
> > I've simplified the patch a bit and fixed up the coding style. Can you please
> > check whether this is still ok from your side?
> 
> I've checked the new patch. It looks like at least the new one (I haven't check
> the older one in this respect) does not compile the master branch of libgee
> even though the Vala master works fine.

libgee master already requires generic accessors. It currently builds with Vala master but it leaks memory, as far as I can tell. Reporting an error instead of leaking memory sounds like the right choice to me.
Comment 13 Jürg Billeter 2012-07-30 15:00:38 UTC
Created attachment 219897 [details] [review]
Updated patch

I found a memory leak, which should be fixed in the updated patch. Can you please check whether it works as expected when using the [GenericAccessors] attribute in libgee master and your branch?
Comment 14 Jeremy Whiting 2012-08-03 21:54:37 UTC
This seems to work as expected on libgee future-abi branch.  libgee unit tests still fail, but they fail on master in the same way, so it's not likely related to this.
Comment 15 Jeremy Whiting 2012-08-03 21:55:47 UTC
I just pushed the future-abi branch to the gnome libgee git repo with my one commit on it that adds GenericAccessors attributes so others can test this also if wanted.
Comment 16 Jürg Billeter 2012-08-06 10:57:21 UTC
commit 6a6a2cf59b7302b0b3b111c6a0c879c00d36ddce
Author: Jeremy Whiting <jpwhiting@kde.org>
Date:   Wed Jul 11 11:54:45 2012 -0600

    Support [GenericAccessors] attribute for interfaces
    
    This adds internal abstract functions to enable access to the element
    type from within a generic interface. These functions are implicitly
    implemented by all classes that implement interfaces with the
    [GenericAccessors] attribute.
    
    Fixes bug 640330.