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 600652 - Problem with nullable variables
Problem with nullable variables
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: general
0.7.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2009-11-04 10:15 UTC by Jan Stępień
Modified: 2010-01-29 17:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
support binary operators with nullable (4.77 KB, patch)
2010-01-17 21:12 UTC, Luca Bruno
none Details | Review
support binary operators for nullable (4.80 KB, patch)
2010-01-17 21:36 UTC, Luca Bruno
none Details | Review
use helper function (6.62 KB, patch)
2010-01-18 21:27 UTC, Luca Bruno
none Details | Review

Description Jan Stępień 2009-11-04 10:15:38 UTC
Following code:

  class Test {
      private int? x;
      public Test(int val) {
          if (x > val) {
              // Nothing
          }
      }
  }

is translated to C code without any warnings by the latest (225d86) valac from Git. While compiling with `gcc -c Test.c` a following warning is outputted:

  Test.c: In function ‘test_construct’:
  Test.c:64: warning: comparison between pointer and integer

Indeed, the generated C code is invalid:

  Test* test_construct (GType object_type, gint val) {
  	Test* self;
  	self = (Test*) g_type_create_instance (object_type);
  	if (self->priv->x > val) {
  	}
  	return self;
  }

valac should either generate correct C code or output an error when trying to compile such instructions.
Comment 1 Luca Bruno 2010-01-17 15:35:45 UTC
int? is a bit weird syntax to declare an owned int pointer.
I agree on the fact that vala should handle this in an error because they're completely different types.
Comment 2 zarevucky.jiri 2010-01-17 16:56:32 UTC
The question mark is a syntax for nullable types, i.e. value types that can be assigned null. It is *not* a pointer.

So, the syntax is not weird and the expression is valid. It's just the C translation that is wrong.


On the topic:
I think that for the sake of comparison, NULL should be less than any non-NULL value.
Comment 3 Jan Stępień 2010-01-17 18:55:22 UTC
(In reply to comment #2)
> On the topic:
> I think that for the sake of comparison, NULL should be less than
> any non-NULL value.

From my point of view null shouldn't be comparable to int/float values as it is not a number. Comparing null and numbers should either return false or throw an exception.
Comment 4 Luca Bruno 2010-01-17 19:15:11 UTC
I'm implementing a behavior that's different between relational operators and arithmetic operators.
For relation operators, we check that if the int? is null, it's not threaten as a number, but really as null.
For arithmetic operators, there's no sense in checking whether a value is null or not: doing a+b is undefined behavior if a is null or b is null. You must check this before doing the operation.

Equality and inequality are simple.
For relational operators, because a >= b means a > b || a == b:
int? a=...;
int? b=...;
a > b => (a != null && b != null && *a > *b)
a >= b => (a == b || (a != null && b != null && *a >= *b))

You can't threat null as a number, because it isn't.

For arithmetic operators, just *a op *b.

Of course, cases where only one of the operand is nullable are ok (like int? - int).
Comment 5 zarevucky.jiri 2010-01-17 19:19:06 UTC
Well, this is not C#, so you can't throw an exception.

The way I see it, we have three possibilities:
1) trigger an error (not too user friendly, as that would mean crash even when the program's function isn't affected by the precise value of null)
2) handle null as value 0 (zero)
3) handle null as something either lesser or greater than any non-null value

Always returning false is IMO not a good idea because it would mean:
(null < 5) == false
(null > 5) == false
(null == 5) == false

That's confusing.
Comment 6 Luca Bruno 2010-01-17 19:23:43 UTC
(In reply to comment #5)
> Well, this is not C#, so you can't throw an exception.
> 
> The way I see it, we have three possibilities:
> 1) trigger an error (not too user friendly, as that would mean crash even when
> the program's function isn't affected by the precise value of null)
> 2) handle null as value 0 (zero)
> 3) handle null as something either lesser or greater than any non-null value
> 
> Always returning false is IMO not a good idea because it would mean:
> (null < 5) == false
> (null > 5) == false
> (null == 5) == false
> 
> That's confusing.

Or, as I commented up, threat null as null. null < 5 is false. null > 5 is false. null <= null is true. null == 5 is false.
It's not confusing. Just threat null to not be a number, but to be null only for pointer equality.
Comment 7 zarevucky.jiri 2010-01-17 19:31:11 UTC
What's the point of comparison with null being valid, if it doesn't have a sort order defined?  I have always considered !(x < y) => (x >= y) being an universal untouchable truth.
Comment 8 Luca Bruno 2010-01-17 19:40:26 UTC
In fact your universal untouchable truth hasn't been touched. Say both are null, then x < y is false while x >= y is true.
If both are not null, then it's the normal int comparison.
If one of them is null then it's always false. Cause null is not a number. What's the point of using int? else?
In other words, I think that null > num or null >= num is always false. If you don't like this, I should use the same concept of arithmetic operations: straight *x op *y and if it's null then undefined behavior.

And null must be valid for comparison at least for equality and inequality, how would check whether an int? is null otherwise?
Comment 9 zarevucky.jiri 2010-01-17 19:49:02 UTC
You are touching my universal truth just by assuming what x and y are. :) No assumptions about that! Ever. Programmer expects that to work with any two values for which it's valid. Breaking it could cause very hard to find bugs in program logic.

So IMO it's either runtime error or fixed sort order. By the way - who ever said null can't behave as a numeric value?
Comment 10 Luca Bruno 2010-01-17 19:52:57 UTC
Cause null is a pointer and can't be an int, and comparison is supposed to not work with that, that's my truth. null < 5 == true makes no sense, that's impossible in any language that I know and it's illogic. I'm going to make the patch do *x and *y without any check for now.
Comment 11 zarevucky.jiri 2010-01-17 20:03:41 UTC
For nullable types, null is not a pointer. It's an empty place. Nothing. What do you get when you compare 0 with nothing?

Anyway, what you say is illogical as well, because you suggest that the comparison of two incomparable values should be valid.

So, you do it without check for now. Let's see what juergbi thinks.
Comment 12 Luca Bruno 2010-01-17 20:33:53 UTC
(In reply to comment #11)
> For nullable types, null is not a pointer. It's an empty place. Nothing. What
> do you get when you compare 0 with nothing?
> 

That's exactly why you can't make it ordered and it can't be threaten like a number.

> Anyway, what you say is illogical as well, because you suggest that the
> comparison of two incomparable values should be valid.

int? can be compared to int?, so it's legal to do a == b at least, or, as I said, you can't check whether it's null.
Maybe it's not legal to do it with int? and int.

> 
> So, you do it without check for now. Let's see what juergbi thinks.

I think it can be left without checks. OtherType? is not being checked when you use it. Programmer must take care of it being not null when used, unless policy for all nullable types change to throw an error.
Comment 13 Luca Bruno 2010-01-17 21:12:21 UTC
Created attachment 151625 [details] [review]
support binary operators with nullable

The patch has been simplified a lot since my first local version, much less invasive.
Anyway I must precise that the int? == null is not handled here, but at a more direct level (a == NULL).
The only thing I've added is this, which sounds rather sane to me:
int? a == int? b => (a == b || (a != null && b != null && *a == *b)
Comment 14 zarevucky.jiri 2010-01-17 21:24:33 UTC
Review of attachment 151625 [details] [review]:

Looks good as far as I can tell :)  (hah, always wanted to try this!)
Comment 15 Luca Bruno 2010-01-17 21:36:12 UTC
Created attachment 151628 [details] [review]
support binary operators for nullable

Was the wrong patch. Sorry for the noise.
Comment 16 Luca Bruno 2010-01-18 21:27:21 UTC
Created attachment 151715 [details] [review]
use helper function

As juergbi said, it's better an helper function (I made it inline cause it's only one row) cause structs are used twice (could mean calling the same function twice in C).
Comment 17 Jürg Billeter 2010-01-29 17:23:48 UTC
I forgot about the patch in this bug again. I've applied an other patch that addresses the same issue as part of fixing bug 608434. If I read your patch correctly, this will cause crashes if you compare a nullable expression with a non-nullable expression and the nullable expression evaluates to null. The patch in master should handle this correctly.

The test case in this bug still doesn't work, though, as this is about relational operators, therefore keeping this bug open.
Comment 18 Jürg Billeter 2010-01-29 17:37:05 UTC
commit d4e96870ebf6ba5a639f3c05e3e5212d363f3521
Author: Jürg Billeter <j@bitron.ch>
Date:   Fri Jan 29 18:35:32 2010 +0100

    Fix relational operations involving nullable types
    
    Fixes bug 600652.