[RFC] Integer signedness story

[gkoz]

Most GTK APIs use gint whenever an integer is needed, largely for C-specific reasons. Since Rust has more strict integer semantics my desire is to use u32 (or u16, or Option<u32> where appropriate) wherever negative values are invalid.

This approach has been applied to a few classes so far: EntryBuffer, Notebook, partly to List/TreeStore.

It may follow that unsignedness needs to be applied as far as the basic structs like GtkAllocation aka GdkRectangle aka cairo_rectangle_int_t (width and height at least). This discussion makes me think it has merit.

Before any further effort is made on this it'd be nice to get some opinions on whether the direction is right or the above classes should be reverted to the upstream C convention (i32 everywhere).

[gkoz]

cc @csoriano89

[csoriano1618]

How are you going to deal with valid unsigned values when they are invalid as gint?
Say if the uint uses the signed bit as a value.

I would say, just follow what gtk+ does to avoid problems with gtk+... (although some decisions might be not the best ones).

[GuillaumeGomez]

It's not unsecure to provide an invalid value to gtk since it'll return an error value. However it's not perfect since rust is supposed to provide full safety. But u32 can allow to have greatest value than i32, so how are we supposed to handle it (that's what @csoriano89 said)?

[gkoz]

How are you going to deal with valid unsigned values when they are invalid as gint?
Say if the uint uses the signed bit as a value.

Catch with asserts probably because typically an out of range value would be a "programmer error". Note that in some cases I have to add some checks anyway, e.g. gtk_tree_store_reorder is plainly unsafe (but maybe it shouldn't have been exposed at all?)

@GuillaumeGomez GTK+ relies on optional asserts to handle various edge cases. If they're not compiled in, those edge cases (even as simple as calling gtk_main_quit outside a main loop) suddenly become unsafe.

[GuillaumeGomez]

@GuillaumeGomez GTK+ relies on optional asserts to handle various edge cases. If they're not compiled in, those edge cases (even as simple as calling gtk_main_quit outside a main loop) suddenly become unsafe.

Bad news.

Catch with asserts probably because typically an out of range value would be a "programmer error". Note that in some cases I have to add some checks anyway, e.g. gtk_tree_store_reorder is plainly unsafe (but maybe it shouldn't have been exposed at all?)

Not necessarily. It's always possible to find cases where a developer isn't directly responsible. However the assert should do the trick. I'm still not a fan of this solution but if we do it, no other choice...

[gkoz]

Another possible alternative is using some U31 type. Not sure if there is a popular implementation, but there's this.

[GuillaumeGomez]

Not sure this is a nice idea to add a new type (even if we use it only internally). But let's keep an eye on it just in case.

[csoriano1618]

"Catch with asserts probably because typically an out of range value would be a "programmer error". "

That's the thing, a big uint value is not an overflow, but the developer will see errors coming from gtk+ about negative values when they used just a guint. Sounds more confusing than what it tries to fix. In case you use a i32 and set a negative value the developer will catch it just printing it, but not a u32.

However if you use a u32 and make some asserts internally with a message explaining to the developer that it's a big value that cannot be processed and the maximum value is 2^31, then why not.

[gkoz]

Yeah, I wouldn't want a negative value to reach gtk+. To be clear the approach I had in mind is

fn get_foo() -> u32 {
    let ret = gtk_get_foo();
    assert!(ret >= 0, "Unexpected negative value");
    ret as u32
}

fn set_foo(val: u32)
    let val = val as c_int;
    assert!(val >= 0, "Value bigger than i32::max_value");
    gtk_set_foo(val);
}

and documenting the presence of this assert.

[csoriano1618]

Then looks like a good idea to me and no downsides on top of my head from the gtk+/c side.

[sdroege]

There doesn't seem to be anything actionable here anymore. To some degree this is done manually now, but as there is no annotation for any such thing it will have to be done manually on a case-by-case basis in the future too.