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 777958 - Invalid C code for abstract classes implementing interface
Invalid C code for abstract classes implementing interface
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator: GObject
0.35.x
Other Linux
: Normal normal
: 1.0
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-01-30 23:45 UTC by Daniel Espinosa
Modified: 2017-02-25 14:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case integrated in Vala's test system (1.59 KB, patch)
2017-02-22 20:41 UTC, Daniel Espinosa
none Details | Review
codegen: Pass pointer of matching type to property-getter (1.08 KB, patch)
2017-02-25 12:20 UTC, Rico Tzschichholz
committed Details | Review
codegen: Cast vfunc pointer assigns of abstract overrides (1.60 KB, patch)
2017-02-25 12:21 UTC, Rico Tzschichholz
committed Details | Review

Description Daniel Espinosa 2017-01-30 23:45:35 UTC
If you have an interface, then you implement it using an abstract class while keeps abstract one method or property defined in interface, produce C code rising warnings.

Best example is libgee, producing warnings when access to size property defined in Collection property and implemented in one of its abstract class.

interface Interface
abstract string a {get;set;}        

abstract class AbstractClass : Object, Interface {
  abstract a {get;set;} 
}

class Class : AbstractClass {
  abstract a {get;set;}
}

When in your code you use:

var c = new Class ();
printf (c.a);

You will get an error like:

/usr/include/gxml-0.14/gxml/gxml.h:5621:8: note: expected 'AbstractClass * {aka struct _AbstractClass *}' but argument is of type 'Interface * {aka struct _Interface *}'
 gchar* abstract_class_get_a (Abstractclass* self);

This is important in order to enable -Wall at compile time.
Comment 1 Daniel Espinosa 2017-02-22 20:05:25 UTC
Example code showing the problem:

public interface Collection : Object {
    public abstract string name { get; set; }
    public abstract bool validate ();
}

public abstract class AbstractCollection : Object, Collection {
    public string name { get; set; }
    public abstract bool validate ();
}

public class HashMap : AbstractCollection {
    public override bool validate () { return true; }
}
Comment 2 Daniel Espinosa 2017-02-22 20:25:50 UTC
This is a workaround:

1) Make your base class *not* implement interfaces but the methods and properties you have in common with other derived classes

2) Your derived classes should implement the rest of methods you need for your interface


public interface Collection : Object {
    public abstract string name { get; set; }
    public abstract bool validate ();
    public abstract string to_string ();
}

public abstract class AbstractCollection : Object {
    public string name { get; set; }
    public string to_string () { return ""; }
}

public class HashMap : AbstractCollection, Collection  {
    public bool validate () { return true; }
}

void main () {}
Comment 3 Daniel Espinosa 2017-02-22 20:30:00 UTC
This is not working if you talk about properties. In next code Base class is not implementing an interface property but derived classed. Now GCC complains about to use incompatible pointers using HashMap* instead of Collection*

public interface Collection : Object {
    public abstract string name { get; set; }
    public abstract int index { get; set; }
    public abstract bool validate ();
    public abstract string to_string ();
}

public abstract class AbstractCollection : Object {
    public string name { get; set; }
    public string to_string () { return ""; }
}

public class HashMap : AbstractCollection, Collection  {
    public int index { get; set; }
    public bool validate () { return true; }
}

void main () {}

Message:

In function ‘hash_map_real_set_index’:
interface-base-abstract.vala.c:316:31: warning: passing argument 1 of ‘hash_map_real_get_index’ from incompatible pointer type [-Wincompatible-pointer-types]
  if (hash_map_real_get_index (self) != value) {
                               ^~~~
interface-base-abstract.vala.c:302:13: note: expected ‘Collection * {aka struct _Collection *}’ but argument is of type ‘HashMap * {aka struct _HashMap *}’
 static gint hash_map_real_get_index (Collection* base) {
             ^~~~~~~~~~~~~~~~~~~~~~~


Tagged for 1.0 because is a compilation warning affecting libgee too.
Comment 4 Daniel Espinosa 2017-02-22 20:41:55 UTC
Created attachment 346497 [details] [review]
Test case integrated in Vala's test system

This patch just add test case to Vala test system, but it PASS and should not because on terminal program returns warnings at GCC compilation time.

interface-base-abstract.vala.c: In function ‘hash_map_real_set_index’:
interface-base-abstract.vala.c:333:31: warning: passing argument 1 of ‘hash_map_real_get_index’ from incompatible pointer type [-Wincompatible-pointer-types]
  if (hash_map_real_get_index (self) != value) {
                               ^~~~
interface-base-abstract.vala.c:319:13: note: expected ‘Collection * {aka struct _Collection *}’ but argument is of type ‘HashMap * {aka struct _HashMap *}’
 static gint hash_map_real_get_index (Collection* base) {
             ^~~~~~~~~~~~~~~~~~~~~~~
interface-base-abstract.vala.c: In function ‘hash_map_class_init’:
interface-base-abstract.vala.c:345:48: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
  ((AbstractCollectionClass *) klass)->validate = hash_map_real_validate;
                                                ^
Comment 5 Rico Tzschichholz 2017-02-25 12:20:58 UTC
Created attachment 346706 [details] [review]
codegen: Pass pointer of matching type to property-getter
Comment 6 Rico Tzschichholz 2017-02-25 12:21:03 UTC
Created attachment 346707 [details] [review]
codegen: Cast vfunc pointer assigns of abstract overrides
Comment 7 Rico Tzschichholz 2017-02-25 12:23:28 UTC
Comment on attachment 346497 [details] [review]
Test case integrated in Vala's test system

Currently it is not possible to have such a test case fail while it requires stricter c-level -Werror settings which would effect the rest of the test-suite too.
Comment 8 Daniel Espinosa 2017-02-25 14:23:42 UTC
Review of attachment 346706 [details] [review]:

It is Ok for me. This + next patch fixes the problem.
Comment 9 Daniel Espinosa 2017-02-25 14:24:07 UTC
Review of attachment 346707 [details] [review]:

It is Ok for me. This + above patch fixes the problem.
Comment 10 Daniel Espinosa 2017-02-25 14:25:13 UTC
Feel free to close when committed.

I'll file a new one for Gee case.
Comment 11 Rico Tzschichholz 2017-02-25 14:41:47 UTC
Attachment 346706 [details] pushed as 8036f87 - codegen: Pass pointer of matching type to property-getter
Attachment 346707 [details] pushed as 64365c2 - codegen: Cast vfunc pointer assigns of abstract overrides