GNOME Bugzilla – Bug 741465
Compact abstract classes with abstract methods generates wrong C code
Last modified: 2018-03-27 10:16:52 UTC
Compact abstract classes generate invalid C code as this example shows: ``` [Compact] public abstract class Test { public abstract void run(); } void main() {} ``` Compilation output is: ``` minimain.vala.c:32:2: warning: implicit declaration of function 'TEST_GET_CLASS' is invalid in C99 [-Wimplicit-function-declaration] TEST_GET_CLASS (self)->run (self); ^ minimain.vala.c:32:25: error: member reference type 'int' is not a pointer TEST_GET_CLASS (self)->run (self); ~~~~~~~~~~~~~~~~~~~~~ ^ 1 warning and 1 error generated. ``` By comenting this TEST_GET_CLASS line, the program compiles. but I guess, compact abstract classes cannot work because it depends on GLib machinery. But it would be good to have support for them.
It's an error to have anything virtual or abstract in compact classes.
(In reply to Luca Bruno from comment #1) > It's an error to have anything virtual or abstract in compact classes. So this should be detected by compiler and rise an error. Features of compact classes are already documented. So we need to close this bug and open a new one related to compiler error reports. https://bugzilla.gnome.org/show_bug.cgi?id=792254
(In reply to pancake from comment #0) > Compact abstract classes generate invalid C code as this example shows: > > ``` > [Compact] > public abstract class Test { > public abstract void run(); > } > void main() {} > > ``` > > Compilation output is: > > ``` > minimain.vala.c:32:2: warning: implicit declaration of function > 'TEST_GET_CLASS' is invalid in C99 [-Wimplicit-function-declaration] > TEST_GET_CLASS (self)->run (self); > ^ > minimain.vala.c:32:25: error: member reference type 'int' is not a pointer > TEST_GET_CLASS (self)->run (self); > ~~~~~~~~~~~~~~~~~~~~~ ^ > 1 warning and 1 error generated. > ``` > > By comenting this TEST_GET_CLASS line, the program compiles. but I guess, > compact abstract classes cannot work because it depends on GLib machinery. > But it would be good to have support for them. Above C generated code is: typedef _Test Test; struct _Test { int dummy; }; static gboolean test_real_run (Test* self) { gboolean _tmp0_ = FALSE; g_critical ("Type `%s' does not implement abstract method `test_run'", g_type_name (G_TYPE_FROM_INSTANCE (self))); return _tmp0_; } gboolean test_run (Test* self) { g_return_val_if_fail (self != NULL, FALSE); return TEST_GET_CLASS (self)->run (self); } I found: a) Test class is never registered in the GType system, as a GBoxed (like GDate), so it is hard to use as a property in GObject classes b) If a) is expected, test_real_run() should avoid to use G_TYPE_FROM_INSTANCE() c) Method test_run() should replace: return TEST_GET_CLASS (self)->run (self); with: return self->run (self);
Created attachment 366527 [details] [review] Initial work to provide support for abstract compact classes Abstract compact classes are required by GLib VAPI in GSource, as it is an abstract compact class with abstract methods. So Vala should allow to generate these kind of classes, allows implement its virtual methods and provide print error messages methods for unimplemented classes, like the ones of GInterfaces. I've just don't find a way to add virtual methods to a compact class declarations, at C generated code. This is a WIP any advice on how to do so, is welcome. This patch just shows advances but no make test pass yet.
Created attachment 366582 [details] [review] Add support for abstract methods in compact classes Vala: Add support for abstract methods in compact classes Adds support to declare an abstract compact class with abstract methods. Methods should be implemented by derived classes. Derived classes are compact class too. Added initialization code to implement abstract classes to printout warnings on unimplemented methods. Derived classes, can override base compact classes' abstract methods. No new fiels are allowed, as always. Abstract properties should be possible, but need to be fixed. So, for now, an error is rised if a property is defined.
Created attachment 366685 [details] [review] codegen: Add support for abstract methods/properties in compact classes
I have cleaned it up a bit and fixed the properties support. Still needs some more verifying.
Created attachment 366686 [details] [review] codegen: Add support for abstract methods/properties in compact classes
Review of attachment 366686 [details] [review]: It's Ok for me. Thanks to fix abstract properties in compact classes. Was really hard to me to find where I need to change the code for them.
Created attachment 366691 [details] [review] codegen: Add support for abstract methods/properties in compact classes
Fixed regression with found with libgee test-suite.
Created attachment 366834 [details] [review] codegen: Add support for abstract/virtual methods and properties in compact classes Based on patch by Daniel Espinosa
(In reply to Rico Tzschichholz from comment #12) > Created attachment 366834 [details] [review] [review] > codegen: Add support for abstract/virtual methods and properties in compact > classes > > Based on patch by Daniel Espinosa I think you should prepare at least two commits: * One with my initital work *as is* * One with all your fixes and additions This is the way I do with my contributors. In the history they will see their names for they work and then authors can push fixes, minor corrections or features additions on the original work. Just push all at the same time, when you are satisfied with the final result. Why I say this? because in the pass you have pushed my work with your own additions while you don't take credit on your work. With this all will be satisfied and credit their work.
@daniel: I see, although I don't want to push something while reverting parts of it in the next commit. I guess you noticed that I ran into a few issues with your initial work. While abstract/virtual methods are very entangled it is hard to split them up properly. If you insist I can set you as author and mention my involvement in the commit message? Note that such discussions up front would be easier over IRC.
@daniel: So it would be great if you start joining the #vala irc channel.
Created attachment 366847 [details] [review] codegen: Add support for abstract/virtual methods and properties in compact classes Reworked and extened by Rico Tzschichholz
Attachment 366847 [details] pushed as 28b4f45 - codegen: Add support for abstract/virtual methods and properties in compact classes
*** Bug 659278 has been marked as a duplicate of this bug. ***