rock segfaults on generic function that redefines a class-level typearg

[thomasfanell]

This example code will cause rock to segfault. As far as I can tell, this problem was introduced after the October 2015 changes in rock.

SomeList: class <T> {
    _backend: T*
    _count := 0
    count ::= this _count
    init: func (capacity: Int) {
        this _backend = gc_malloc(capacity * T size)
    }
    add: func (item: T) {
        this _backend[this _count] = item
        this _count += 1
    }
    // this is the culprit
    map: func <T, S> (function: Func(T) -> S) -> This<S> {
        result := This<S> new(this _count)
        for (i in 0 .. this _count)
            result add(function(this[i]))
        result
    }
    operator [] (index: Int) -> T {
        this _backend[index]
    }
}

someList := SomeList<Int> new(3)
someList add(1); someList add(2); someList add(3)
// here we segfault
newList := someList map(|value| value + 1)

for (i in 0 .. newList count)
    "%d" printfln(newList[i])

If I change map to this:

    // qualify S
    map: func <T, S> (S: Class, function: Func(T) -> S) -> This<S> {
        result := This<S> new(this _count)
        for (i in 0 .. this _count)
            result add(function(this[i]))
        result
    }

and the map call to: newList := someList map(Int, |value| value + 1), it works fine.

[alexnask]

I will be taking a look and fixing this tomorrow.
Thank you for the report.

[thomasfanell]

This seems to work, for both the newer version and the pre-October version.

// instead of func <T, S>
map: func <S> (function: Func(T) -> S) -> This<S>

[alexnask]

Ah, the problem could actually be the redefinition of the generic argument (although I would expect it to be shadowed or error out).

Are you positive the original code worked before the generic inference changes?

Either way, I'll probably take a look at the cause this evening and expect a fix tomorrow.

[thomasfanell]

I'm 100% positive that rock did not segfault, but I'm only 90% sure that the code actually worked. I does work on our current version of rock, which is based on 0.9.10. I've juggled many different versions of rock today, so there is a slight chance I might have gotten something wrong, although I don't think so.

A fresh pair of eyes would be appreciated!

[alexnask]

Ok, thank you!

I'm pretty surprised no test or rock code does something similar but I guess it's a good opportunity to add similar tests.

[alexnask]

Let's establish what should happen first:

In my opinion this should either error out because we are trying to re-use T or shadow T and throw the classic " Missing info for type argument T" error.

It should not segfault for sure though :P

[alexnask]

Here is the simpler test I will be using, by the way:

Foo: class <T> {
    val: T

    init: func (=val)

    bar: func <T, S> (f: Func (T) -> S) -> This <S> {
        This new(f(val))
    }
}

foo := Foo new(42)
foo bar(|i| i toString())

[alexnask]

@thomasfanell Either way, this code is not correct, so you should find out where it happens in your codebase and get rid of it.

Of course I will fix the segfault and preferably have it throw a warning, I just noticed you said it works on your modified 0.9.10 rock so I want to make clear it shouldn't :P

[thomasfanell]

We have modified the code that is causing the segfault, so the project compiles fine now (we're not using that code at the moment anyway).

Having rock print a warning regarding stuff like this would be very nice! I would save us many headaches I think. I have to tell you that I am somewhat confused at this point. What is supposed work then? This is one of your examples from the ooc site:

map: func <T, U> (list: List<T>, f: Func (T) -> U) -> List<U>

Is this the only way of doing it? Shipping the list we're mapping on as an argument?

mappedList := list map(list, |value| value + 1)

[alexnask]

@thomasfanell T is already defined by the class and is accessible at any point inside its scope.

Redefining it inside the class should work but shadow the original definition (thus, with the current map signature that is causing trouble, throwing an error that T has missing type info for any call, since there is no way to get its type from a closure)

The examples on the ooc site is (presumably) outside the class scope, so it is perfectly valid (and obviously needed)

I don't know if I am explaining this clearly enough, feel free to ask if something is unclear :)

[alexnask]

Think of any generic type as a type alias scoped to the kind it is attached to.

For example class <T> means that T is attached to the scope of the class, while func <T> means T is attached to the scope of the function.

If the function is inside the class, then you are shadowing (redefining) T in the scope of the function (which is not invalid but can be confusing), not telling the compiler to re-use the class' T.

[thomasfanell]

Ah, I just assumed that T would be whatever type was used when declaring the class instance.

So, this should be correct(?):

map: func <S> (function: Func(T) -> S) -> This<S>

Here is another one:

// error Not enough info to resolve return type S of function call
fold: func <S> (function: Func(T, S) -> S, initial: S) -> S
// this works
fold: func <S> (S: Class, function: Func(T, S) -> S, initial: S) -> S

In the map() case, S is clearly known, but in the fold() case, it's not?

[alexnask]

The map signature is correct.
The fold signature ought to work too, rock has a bit of trouble with generic inference though.

(In fact, I don't really see why it doesn't it should be able to deduce S from both the first and the second argument)

I should rewrite the whole thing at some point...

[thomasfanell]

So we're required to specify S like this then... far from optimal but I guess it's not the end of the world. :-)

I appreciate you taking the time to clear this stuff up!

[alexnask]

No problem, it's really our fault you are having so much trouble, rock should have been able to produce correct errors and warnings about that.

One last little note, be careful if you ever try to pass both a base and child class as a generic, the base one will not necessarily be chosen as is.

For example, in the first fold signature, if you passed a function where the returned type was Child and then passed an initial object of type Base, Child would be assigned to S (assuming rock did its job).

This should be fixed by getting all candidates and finding their common root at some point.

[thomasfanell]

Thanks for the heads-up, I will note that down :+1:

[fasterthanlime]

@shamanas @thomasfanell does swapping Func(S,T) and initial: S in the function signature make current-rock successfully infer S ?

[thomasfanell]

@fasterthanlime I actually tried that earlier, no go though :(

[fasterthanlime]

@thomasfanell alright, worth a try. Since current-rock is kinda dumb and will try to resolve them in-order rather than easiest-first, I thought it might choke on Func(S,T) and not even try S.

[alexnask]

Thankfully, rock is not that dumb, it only chokes after having tried to infer the typeArg from all arguments :P

This case is still really surprising though