GNOME Bugzilla – Bug 703804
Valac 0.22 introduced warnings fixing which produces invalid C code
Last modified: 2013-07-14 07:37:40 UTC
Following code: [CCode (scope = "async")] public delegate void Run(); static void eval(Run run) { run (); } int main() { int i = 0; eval(() => { i++; }); return 0; } produces warnings in Vala despite resulting in correct code: static void ___lambda2__run (gpointer self) { __lambda2_ (self); block1_data_unref (self); } gint _vala_main (void) { gint result = 0; Block1Data* _data1_; _data1_ = g_slice_new0 (Block1Data); _data1_->_ref_count_ = 1; _data1_->i = 0; eval (___lambda2__run, block1_data_ref (_data1_)); result = 0; block1_data_unref (_data1_); _data1_ = NULL; return result; } If the code is 'fixed' and delegate is owned it results in incorrect C code: /tmp/test2.vala.c:73:2: error: too few arguments to function ‘eval’ eval (___lambda2__run, block1_data_ref (_data1_)); ^ /tmp/test2.vala.c:26:6: note: declared here void eval (Run run, void* run_target, GDestroyNotify run_target_destroy_notify) { ^ void eval (Run run, void* run_target, GDestroyNotify run_target_destroy_notify) { Run _tmp0_ = NULL; void* _tmp0__target = NULL; _tmp0_ = run; _tmp0__target = run_target; _tmp0_ (_tmp0__target); (run_target_destroy_notify == NULL) ? NULL : (run_target_destroy_notify (run_target), NULL); run = NULL; run_target = NULL; run_target_destroy_notify = NULL; }
Basically because scope="async" was only meant for bindings. It requires additional code to make it work for new generated code.
(In reply to comment #1) > Basically because scope="async" was only meant for bindings. Hmm. Such type of information should probably be included in the manual (https://live.gnome.org/Vala/Manual/Attributes). > It requires additional code to make it work for new generated code. Hmm. Why? This worked just fine out of the box. From callee perspective it behaved as unowned function - no need to free it. From caller perspective it is owned value (the function owns it) so it needs to take care about the cleanup (at least this is the explanation I heard somewhere - I cannot find where atm). I'm not sure why it was changed - especially that historically (in 0.20 - glib-2.0.vapi) the function was unowned.
> > Hmm. Such type of information should probably be included in the manual > (https://live.gnome.org/Vala/Manual/Attributes). > > > It requires additional code to make it work for new generated code. > > Hmm. Why? This worked just fine out of the box. From callee perspective it > behaved as unowned function - no need to free it. From caller perspective it is > owned value (the function owns it) so it needs to take care about the cleanup > (at least this is the explanation I heard somewhere - I cannot find where atm). > It must be owned because the callee must not keep a reference to it anymore. It is really owned by the caller, the fact that it has no more a destroynotify is secondary, and the fact that is the caller that takes care of the cleanup, it doesn't do it at the time when it passes the callback, it does it in another time. Thus, that variable is no more owned by the caller in that context. Not sure if I've been clear enough in the explanation. > I'm not sure why it was changed - especially that historically (in 0.20 - > glib-2.0.vapi) the function was unowned. It has always been wrong.
Ehm, let me rephrase, mixed callee and caller: It must be owned because the caller must not keep a reference to it anymore. It is really owned by the callee, the fact that it has no more a destroynotify is secondary. About the fact that is the caller that takes care of the cleanup, it doesn't do it at the time when it passes the callback, it does it in another time. Thus, that variable is no more owned by the caller in that context. Not sure if I've been clear enough in the explanation.
Created attachment 248966 [details] [review] codegen: Allow scope=async delegates to be used in vala code Fixes bug 703804 Does it make your code work and tests pass? It's a quite important change.
For some reasons the ownership cannot be transferred. (BTW. It is likely that I will remove the scope=async to handle errors properly.)
Created attachment 249043 [details] [review] codegen: Allow scope=async delegates to be used in vala code Fixes bug 703804
(In reply to comment #7) > Created an attachment (id=249043) [details] [review] > codegen: Allow scope=async delegates to be used in vala code > > Fixes bug 703804 It looks like it is working now.
commit 6d07669384cdb70c3c657dba67d5048212f25da9 Author: Luca Bruno <lucabru@src.gnome.org> Date: Thu Jul 11 22:39:53 2013 +0200 codegen: Allow scope=async delegates to be used in vala code Fixes bug 703804 I fear this change, but let's try :-)
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.