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 591673 - Out parameters should be guarded and allowed to be null by default
Out parameters should be guarded and allowed to be null by default
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Vala maintainers
Vala maintainers
: 584575 619153 (view as bug list)
Depends on:
Blocks: 591666
 
 
Reported: 2009-08-13 11:27 UTC by Maciej (Matthew) Piechotka
Modified: 2010-10-16 10:42 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Maciej (Matthew) Piechotka 2009-08-13 11:27:36 UTC
Out parameters should be guarded. I.e.
void method(out Type value) {
    value = something();
}

should result in something like:
void method(Type *t) {
    Type tmp0 = something();
    if(t != NULL)
        t = tmp0;
    else
        type_free(tmp0);
}
Comment 1 zarevucky.jiri 2009-12-22 18:44:28 UTC
I don't see the benefit of having Vala do this. When you want the out parameter to be nullable, you have to make it explicit as with all the other parameters. When you do have it explicitly nullable, you will naturally take care of the null case yourself. That is the common scenario and having Vala generate these checks would be totally useless.

On the other hand, once all the big stuff is finished and fixed (let's say.. in a few years, in case we're still here after 2012), it wouldn't be vain to have a new cool flow analyzer check that the programmer is handling this correctly. :)
Comment 2 Marc-Andre Lureau 2010-01-18 01:25:16 UTC
(In reply to comment #1)
> I don't see the benefit of having Vala do this. When you want the out parameter
> to be nullable, you have to make it explicit as with all the other parameters.
> When you do have it explicitly nullable, you will naturally take care of the
> null case yourself. That is the common scenario and having Vala generate these
> checks would be totally useless.
> 

I agree with Jiří. Maciej did you changed your mind? or do you still think it would be required?

for me there is still an ambiguity with void method(out Type? value), does that means that argument is optional or that it may return null? 

For reference, I think the current code generated with a nullable argument is a bit funny: 

        if (value != NULL) {
                *value = NULL;
        }
        *value = (_tmp0_ = type_new (), _type_unref0 (*value), _tmp0_);
Comment 3 Maciej (Matthew) Piechotka 2010-01-19 00:04:49 UTC
Hmm. I guess that:
- if user of library wants to ignore the results he will. I wrote such code as (althought it was not production code) in C#:
    SomeVar v;
    SomeMethod (out v);
    // rest of code never mentioning v
Instead of:
    SomeMethod (null);
- The problem that the out parameter cannot be explicitly null as mentioned by Marc-Andre. If void method(out Type? value) means that the out parameter is optional how to say that it may throw out null? It is waste of syntax as null means only that user is not interested in output (and if he is not then he is not) - not that it accepts null or can return null [so the ignoring property is not the property of function but some low-level stuff).
- writing (for libgee) n-th time:
    if (out_parameter != null)
        out_parameter = ...
when I could just write:
    out_parameter = ...
Comment 4 zarevucky.jiri 2010-01-19 13:35:29 UTC
The problem is that not all out parameters can be null. Remember that Vala strives to be compatible with existing C code. So you have to define whether you can or cannot pass null, otherwise you'd be very very evil to people who don't enjoy reading documentation, and also inconsistent with the rest of parameter types. 

The syntax is a little ambiguous, that I agree with. Maybe there it should be differentiated, whether the parameter's type is nullable (currently it always behaves as a normal local variable I think), or whether the parameter itself is nullable.

For example:
   void get_some_thing (out? int num);
vs.
   void get_some_thing (out int? num);
vs.
   void get_some_thing (out? int? num);

This way, it would be obvious that the argument can or can't be null, and int? would mean variable of type "int?", not optional variable of type "int".

And why not having Vala check whether the argument isn't null? Because you will want to do that yourself in many cases. If the caller doesn't need the value, you don't need to do any action that is only needed for that retrieval. Vala can't provide that kind of logic, so you will naturally want to optimize that case yourself. It would create redundant and bloated C code, if Vala did those checks for you. That could be solved by an attibute, for example:

   void get_some_thing ([Unchecked] out? Object sth);

I think that would be acceptable in case people really mind manual checking that much.

By the way, I don't know whether it's a typo in you comment, so just to be sure - you have to check like this:
if (&out_parameter != null) {
    out_parameter = ...
}

By simply checking "out_parameter != null" you are testing the assigned value, which would mean crash if the reference is null.
Comment 5 Jürg Billeter 2010-01-29 19:23:14 UTC
(In reply to comment #2)
> for me there is still an ambiguity with void method(out Type? value), does that
> means that argument is optional or that it may return null? 

This means that it may return null. Currently, all output parameters are considered optional in the sense that you can pass null if you're not interested in the result.

To avoid crashes when calling C functions that don't accept NULL, we might want to introduce an attribute that can be used in bindings. I don't think adding syntax along the line of `out?` is worth it, though, as it's not very interesting from a semantic point of view (out values are always ignorable if you don't mind declaring a variable that you don't use).

To avoid crashes in Vala methods that have output parameters, I propose a slightly different implementation. Instead of adding a NULL check for every assignment, I'm planning to add a local variable for every out parameter and only assign to the actual out parameter when returning from the method. At that point, a NULL check is necessary. This is similar to the `result` variable that is currently used for return values except for the NULL check.

An alternative would be to handle null on the caller side by implicitly adding temporary variables. However, this would be less in line with what we're used to with GLib-based libraries and it would be inconvenient when using Vala libraries from C.
Comment 6 Jürg Billeter 2010-01-29 21:11:22 UTC
*** Bug 584575 has been marked as a duplicate of this bug. ***
Comment 7 Jürg Billeter 2010-06-16 19:51:28 UTC
*** Bug 619153 has been marked as a duplicate of this bug. ***
Comment 8 Allison Karlitskaya (desrt) 2010-08-08 20:52:20 UTC
We just had a conversation on IRC about the confusion between passing a C NULL to an out parameter and having Vala nullable types appearing as out parameters.

We came to these conclusions about "the way it should be":

  void foo (out string? x);

means that the we have an out parameter with a type of 'string?'.  It is perfectly valid for x to have a value of 'null' when the function returns.  The question mark here has nothing to do with the validity of omitting a variable to store the result into or if it is valid to do so.

similarly:

  void foo (out string x);

simply means that on return, the value of 'x' will be non-NULL.  Again, no statement is made about if it is possible to omit a variable to store the result.



At the same time: it should always be possible to omit a variable to store an 'out' result into.  A new keyword like ('ignore') would be preferable here:

  foo (out x);

or

  foo (ignore);


because it avoids the confusion associated with using 'null' for something completely unrelated to nullability.  At the same time though, for practical and compatibility reasons, it may be desirable to allow 'null' to be used here.

Of course, 'ignore' (or unfortunately 'null') would be handled by passing NULL in C (but that's an implementation detail unrelated to Vala's 'null').


On the callee side, we handle the situation by having a local variable to shadow every out parameter.  On entry to the function, we check if the out parameter has been passed C NULL and set it to point to the local variable if so.  All access is made like normal and then the local variable is cleared when the function is about to return.

There is some agreement that it should not be possible to 'ignore' ref parameters.
Comment 9 Allison Karlitskaya (desrt) 2010-08-08 20:58:50 UTC
Bug 626395 is the related issue for gobject-introspection.
Comment 10 Jürg Billeter 2010-10-16 10:42:16 UTC
commit 2444fce618821a1a20fd0549da5b3981574654cd
Author: Jürg Billeter <j@bitron.ch>
Date:   Sat Oct 16 12:26:16 2010 +0200

    codegen: Guard access to out parameters to allow null arguments
    
    Fixes bug 591673.