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 673389 - Abstract protected construct properties defined in an interface generate static C setter method that cant be accessed from implementing classes in another vala files
Abstract protected construct properties defined in an interface generate sta...
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
0.16.x
Other Linux
: Normal major
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2012-04-02 22:05 UTC by jmcastellano
Modified: 2017-02-24 13:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A preliminary patch that avoids producing static declarations for abstract property accessors. (1.97 KB, patch)
2015-07-24 06:39 UTC, Jordan Yelloz
needs-work Details | Review
re-formatted version of original patch, >1 year later (1.93 KB, patch)
2017-02-22 23:51 UTC, Jordan Yelloz
none Details | Review
codegen: Don't add static modifier to abstract property setters (1.81 KB, patch)
2017-02-23 22:00 UTC, Rico Tzschichholz
committed Details | Review

Description jmcastellano 2012-04-02 22:05:02 UTC
If you declare an interface with a property with a protected construct in one file

--- test-interface-construct.vala -----
public interface TestInterface : GLib.Object {
    public abstract int property { public get; protected construct; }
}

And an implementation of that interface in a different file

---- test-interface-construct-class.vala -----
public class TestClass : GLib.Object, TestInterface {

    public int property { public get; internal construct; }
}

public static void main() {
    TestClass tc = (TestClass)Object.new(typeof(TestClass), "property", 5);

    stdout.printf("Constructed object property value: %d\n", tc.property);
}


The generated code produces an error (C compiler)

test-interface-construct-class.vala.c:65:13: warning: ‘test_interface_set_property’ used but never defined [enabled by default]
/tmp/cccxA7tn.o: In function `_vala_testclass_set_property':
test-interface-construct-class.vala.c:(.text+0x38e): undefined reference to `test_interface_set_property'
collect2: ld returned 1 exit status
error: cc exited with status 256


The reason is that the setter for the property generated by the interface is declared as a static C function that cannot be found by the implementing class set_property method
Comment 1 Jordan Yelloz 2015-07-24 06:39:47 UTC
Created attachment 308053 [details] [review]
A preliminary patch that avoids producing static declarations for abstract property accessors.

I've looked into this problem and found what I believe is the cause. I made a quick patch to fix it but who knows if it is breaking something else?

Essentially, my assumption is that the code should not generate static method declarations/definitions when working on an abstract property's accessors since somebody might implement those concretely in a separate source code module.
Comment 2 Jordan Yelloz 2015-07-24 07:12:18 UTC
Review of attachment 308053 [details] [review]:

This appears to have been the wrong approach. I think the right approach is to prevent vala from emitting a reference to the setter in the implementation's _vala_*_property_set method.
Comment 3 Jordan Yelloz 2015-07-24 07:22:13 UTC
Review of attachment 308053 [details] [review]:

After running some tests, it appears to have been the correct approach. The approach of evoiding emitting set-accessors breaks object construction. Exporting the interface's property set-accessor symbol allows the concrete implementations to be constructed and GObject rejects future modifications of the property value during runtime.
Comment 4 bob 2017-01-28 03:18:13 UTC
This appears to be the same as https://bugzilla.gnome.org/show_bug.cgi?id=678065
Comment 5 bob 2017-01-28 03:19:54 UTC
D'oh, I forgot to mention that I can replicate on 0.34.4. Sorry for the noise
Comment 6 Daniel Espinosa 2017-02-22 23:33:21 UTC
Review of attachment 308053 [details] [review]:

This patch no longer apply. Please re-work it and attach new one.
Comment 7 Jordan Yelloz 2017-02-22 23:51:14 UTC
Created attachment 346521 [details] [review]
re-formatted version of original patch, >1 year later

I have not tested this but it appears to be the same as the old patch, which I tested back then.
Comment 8 Jordan Yelloz 2017-02-23 18:40:25 UTC
I've tested my patch against 1cc30cc2c0fd5ab440cd948b2f21b247118f9790 and the original testcase works with my patch and does not work without the patch.
Comment 9 Rico Tzschichholz 2017-02-23 22:00:08 UTC
Created attachment 346607 [details] [review]
codegen: Don't add static modifier to abstract property setters
Comment 10 Rico Tzschichholz 2017-02-23 22:27:23 UTC
@jordan: The attached patch feel more fitting to avoid the "static" case.
Comment 11 Jordan Yelloz 2017-02-23 22:57:04 UTC
@rico: I'm fine with your version.
Comment 12 Rico Tzschichholz 2017-02-24 13:30:26 UTC
Attachment 346607 [details] pushed as d1a3c5c - codegen: Don't add static modifier to abstract property setters