Parameter of type Object matches first type of Cell<T> regardless of T's type #802

Closed
davidhesselbom opened this Issue Aug 19, 2014 · 28 comments

Projects

None yet

4 participants

@davidhesselbom
Contributor
write: func(data: Object) {
    (match (data) {
        // This below case matches Cell<T> for any T, which is a problem related to how generic types are implemented
        case c: Cell<String> =>
            "To get here I should be a Cell<String> but apparently I'm a " + c class name + "<" + c T name + ">"
        case cell: Cell<Int> =>
            cell get() toString()
        case =>
            "FAIL"
    }) println()
}
write(Cell new(2))

outputs

To get here I should be a Cell<String> but apparently I'm a Cell<Int>

@davidhesselbom davidhesselbom changed the title from parameter of type Object matches first type of Cell<T> regardless of T's type to Parameter of type Object matches first type of Cell<T> regardless of T's type Aug 19, 2014
@shamanas
Collaborator

Ah, this is a nice one.

Basically the matches against types are calls to instanceOf? (or inheritsFrom?, not too sure, depends on whether they are called on the object or its class, either way the bug is the same) but all instanceOf? does is recursively check the class, class super (etc...) against the type provided.

That means that generic types are never checked against, only the base type.
I think one solution would be to generate a different instanceOf? for types with generics which would also check that the generic types match (or are instances of each other)

@fasterthanlime, any thoughts?

@fasterthanlime
Collaborator

The parameter that 'instanceOf?' receives doesn't have generic type
information anyway, so I don't think it's possible.

A nested match on 'T' is not too much hassle

On Tue, Aug 19, 2014 at 3:05 PM, Alexandros Naskos <notifications@github.com

wrote:

Ah, this is a nice one.

Basically the matches against types are calls to instanceOf? (or
inheritsFrom?, not too sure, depends on whether they are called on the
object or its class, either way the bug is the same) but all instanceOf?
does is recursively check the class, class super (etc...) against the type
provided.

That means that generic types are never checked against, only the base
type.

I think one solution would be to generate a different instanceOf? for
types with generics which would also check that the generic types match (or
are instances of each other)

@fasterthanlime https://github.com/fasterthanlime, any thoughts?


Reply to this email directly or view it on GitHub
#802 (comment).

@simonmika

Although almost impossible to change now would it not be better to have the class store the type variable instead of every object and create a separate class at runtime for every combination of type variables?

@shamanas
Collaborator

Hmm yes, indeed, the Class instancehas no information on the generics.

The way ooc generics work, I think it actually makes sense that the instance variable contains the generic information rather than building a separate metaclass for each combination used (why not do templates at that point?), however we do run into this issue...

@shamanas
Collaborator

Anyway, I think a warning is the way to go at that point, perhaps this could be reconsidered in the future when I start oc (I intend to fully document every feature, the runtime and its C implementation at that point).

@simonmika

And the current solution bloats all instances of a generic type.
But it is probably to late to change.

@shamanas
Collaborator

Actually, I think it would be possible to hack something together.

Rather than changing where the information on generics is held and instaceOf? / inheritsFrom?, we could add conditions checking the generic types on case clauses that have generic types.

E.g.

case c: Cell<String> =>
// becomes
case c instanceOf?(Cell) && c T instanceOf?(String) =>
// rather than
case c instanceOf?(Cell) =>
@fasterthanlime
Collaborator

@shamanas: it's a language-level feature, not a compiler-level feature.

Right now, the language says (although, again, we have no proper spec) that
match only works on base types, not generic type parameters.

It also says you can explore a generic type instance's type parameters via
their name, e.g. "list T", but it doesn't say you can list the type
parameters of a "class" that's passed as parameter (as is the case with
generics)

I've wanted to build more introspection into ooc for a long while but it's
a very non-trivial change. The most intuitive data structure to store type
parameters would be an (ordered) hashmap - but hashmap isn't imported by
default, and having an hashmap instance in the Class class would lead to C
compilers losing their shit in no time, because of initialization order,
etc.

Another solution would be an ad-hoc solution, such as a linked list (not
the one from structs/) but then it's awkward to duplicate types and not
have it implement Iterable&co, like you'd expect it to.

On Tue, Aug 19, 2014 at 3:31 PM, Simon Mika notifications@github.com
wrote:

And the current solution bloats all instances of a generic type.


Reply to this email directly or view it on GitHub
#802 (comment).

@simonmika

That is a cleaver solution that does not break anything, almost at least. Is it possible to also support the following:

case c: Cell<_> =>
// becomes
case c instanceOf?(Cell) =>
@simonmika

Or maybe this would do (if it is not possible to create another type Cell):

case c: Cell =>
// becomes
case c instanceOf?(Cell)
@shamanas
Collaborator

@fasterthanlime
Sure but I see no reason not to change that definition of match on language level when it can be achieved easilly by rock.
The language spec doesn't have to impose the way match compares generic types and more introspection would be nice but we can achieve this (and it actually makes sense) right now.

@simonmika
I think your second example should already work

Edit: Nope, it doesn't, it seems it is imposed that generic types are complete
However, c: Cell<Object> should be a catch-all

@fasterthanlime
Collaborator

Doesn't

case: Cell =>

already work?

then you can access 'case T' - that's what I was hinting at in my first
answer.

The difference with (case instanceOf?(Cell)) is that match with a variable
decl does the casting for you, so you can immediately use it.

On Tue, Aug 19, 2014 at 3:43 PM, Alexandros Naskos <notifications@github.com

wrote:

@fasterthanlime https://github.com/fasterthanlime

Sure but I see no reason not to change that definition of match on
language level when it can be achieved easilly within rock.

The language spec doesn't have to impose the way match compares generic
types ans more introspection would be nice but we can achieve this (and it
actually makes sense) right now.

@simonmika https://github.com/simonmika
I think your second example should already work


Reply to this email directly or view it on GitHub
#802 (comment).

@shamanas
Collaborator

@fasterthanlime
Nope, gives "No type parameters for Cell. It should match Cell"

Yeah, it is still possible to achieve this by matching against Cell<Object> and then a match/if for c T but I think it makes sense for match to be able to do that automatically and cast to the correct type which as you said is the nice thing about this syntax.

@simonmika

No this fails at compile time:

write: func(data: Object) {
    (match (data) {
        case cell: Cell =>
            cell get()
        case =>
            "FAIL"
    }) println()
}
write(Cell new(2))

With the following error:

genericType.ooc:5:14 error No type parameters for Cell. It should match Cell
        case cell: Cell =>
@shamanas
Collaborator

@fasterthanlime

So what do you reckon the final verdict is?
Personally, I think that the generic type tests should be added but its up to you, should I add this behavior or a warning?

@shamanas shamanas self-assigned this Aug 20, 2014
@fasterthanlime
Collaborator

I honestly don't know.

However, there should be a way to match a type that has generic parameters
without specifying the type parameters. Beyond that, I still think it's a
better idea to nest matches by hand.

On Wed, Aug 20, 2014 at 5:36 PM, Alexandros Naskos <notifications@github.com

wrote:

@fasterthanlime https://github.com/fasterthanlime

So what do you reckin the final verdict is?
Personally, I think that the generic type tests should be added but its up
to you, should I add this behavior or a warning?


Reply to this email directly or view it on GitHub
#802 (comment).

@shamanas
Collaborator

Ok so warning + make

case foo: Foo => ...

acceptable, when Foo has generic parameters.

@fasterthanlime
Collaborator

I was thinking perhaps Foo<?> just to make sure. Not sure.

On Thu, Aug 21, 2014 at 10:20 AM, Alexandros Naskos <
notifications@github.com> wrote:

Ok so warning + make

case foo: Foo => ...

acceptable, when Foo has generic parameters.


Reply to this email directly or view it on GitHub
#802 (comment).

@shamanas
Collaborator

Yeah, that makes sense to indicate the type is generic but it means special variable definition grammar for case clauses.

@shamanas
Collaborator

@fasterthanlime
How about making something like this:

match obj {
    case c: Cell<type> =>
        // c is now the Cell variable, type is now c T
        match type {
            // ...
        }
}

This would also allow for Foo<_> syntax when we don't need to access the type, since _ is a valid identifier.

@davidhesselbom
Contributor

Beyond that, I still think it's a better idea to nest matches by hand.

Can you give an actual example that allows me to match on the T in Cell<T> after I've matched on Cell?

write: func <T> (data: Object) {
    (match (data) {
    // "case value: Cell =>" won't build because "No type parameters for Cell. It should match Cell",
    // as already mentioned, so doing "Cell<Object>" instead
        case value: Cell<Object> =>
            (match (value T) {
                case valTee: String =>
                    value get()
                case valTee: Int =>
                    value get() toString()
            })
        case =>
            "FAIL"
    }) println()
}

This won't build because "No such function toString() for Object". Fair enough, but if I change it to

case value: Cell<String> =>
    (match (value T) {
    case valTee: String =>
        value get()
    case valTee: Int =>
        value get() as Int toString()

I instead get the error Missing info for type argument T. Have you forgotten to qualify write<T> (data : Object :Object), e.g. List<Int>? on the call write(Cell new(2)). This may very well be a legit error, but I don't know how to fix it. It sounds to me as if I'm supposed to write write<Cell>(Cell new(2)), which I'm pretty sure was exactly what you said you didn't like about how other languages do generics. It also makes rock segfault... :)

Ok, so let's ignore the function that takes an Object for now, and try something simpler: A function that takes Cell<T>:

write: func <T> (data: Cell<T>) {
    (match (data T) {
        case value: String =>
            data get() as String
        case value: Int =>
            data get() as Int toString()
        case =>
            "FAIL"
    }) println()
}
write(Cell new(2)) // prints "FAIL"
write(Cell new("Hello")) // also prints "FAIL"

Well, that didn't work.

What about (Cell<T>, T)?

write: func <T> (data: Cell<T>, t: T) {
    (match (data T) {
        case value: String =>
            data get() as String
        case value: Int =>
            data get() as Int toString()
        case =>
            "FAIL"
    }) println()
}

Well, C compiler failed on module testtestest from testtestest, bailing out. Obviously, I have no idea what I'm doing anymore. Help?

@fasterthanlime
Collaborator

It's kind of hard to read through all that so it's entirely possible that I missed soemthing, but I noticed something.

You can either do

cell: Cell<T>
match (cell T) {
  case Int =>
    i := cell get() as Int
  case Float =>
    f := cell get() as Float
}

(which is long-form), or you can do the short form:

cell: Cell<T>
match (cell get()) {
  case i: Int =>
    // whatever
  case f: Float =>
    // whatever
}

But above, you're conflating the two, so there's no chance it'll work :) Generic values & generic types aren't the same thing.

@fasterthanlime
Collaborator

@davidhesselbom here's a full example that works well for me:

Whatever: class { init: func }

write: func ~noCell <T> (t: T) {
    write(Cell new(t))
}

write: func <T> (cell: Cell<T>) {
    match (cell get()) {
        case i: Int =>
            "Got int #{i}" println()
        case f: Float =>
            "Got float #{f}" println()
        case =>
            "Got other type #{cell T name}" println()
    }
}

write(42 as Int)
write(3.14 as Float)
write(Whatever new())
@fasterthanlime
Collaborator

(And indeed, it's a pity that the second write here needs a <T>, it shouldn't. I think Cell<?> syntax should exist. The compiler check for unqualified generic types is probably too severe, but believe me, it did a lot of damage control back in the days...)

@davidhesselbom
Contributor

Splendid, thank you sir!

@fasterthanlime
Collaborator

No problem. Sorry it took a long-ass issue for this info to get out. It'd be probably right at home somewhere at https://github.com/fasterthanlime/ooc-lang.org/blob/master/content/docs/lang/generics.md, perhaps in a more documentation-y formulation. If someone has time, don't hesitate to send in a pull request.

@davidhesselbom
Contributor

I'll look into it. Meanwhile, I gave you another, much smaller one :)

@fasterthanlime fasterthanlime added a commit that closed this issue Jul 10, 2015
@fasterthanlime fasterthanlime Closes #802 1112d9a
@fasterthanlime
Collaborator

So, matching the actual typeArgs should be another issue (if at all), but matching generic types without specifying typeArgs at all is now possible.

@fasterthanlime fasterthanlime modified the milestone: 0.9.10 Jul 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment