Closed alexnask closed 8 years ago
Basically, things like this didn't work (at the call site, the declaration was ok):
extend Cell <NewGenName> {
id: func -> NewGenName {
val
}
}
c := Cell new(42)
i := c id() // This failed to realtypize NewGenName
@shamanas would you mind rebasing?
@thomasfanell
Here you go
Grazie!
ExampleClass: class <T> {
value: T
init: func (=value)
}
extend ExampleClass<T> {
getTypeName: func -> String { T name }
}
"%s" printfln(ExampleClass<String> new("moo") getTypeName()) // "String"
"%s" printfln(ExampleClass<String> new(null) getTypeName()) // "Pointer"
"%s" printfln(ExampleClass<String> new(null as String) getTypeName()) // "String"
I would expect T
to be String
in all three of these cases. This has really nothing to do with extending generic classes, as moving getTypeName
into the class definition yields the same result, it's just a general concern, and I can see some potential problems with this.
@thomasfanell
Yes, I think this is indeed a bug, since ExampleClass is fully qualified.
I will check it out and hopefully have a fix, I think it will be pretty easy (rock probably ignores that it is fully qualified and goes on to resolve the generic itself when instead it should just check if the types are compatible).
By the way, up until now I just brought fixes like that to ooc-lang/rock, would you like me to open PRs on magic-lang too?
(There is already one bug fix that went through on ooc and not magic, #983)
rock probably ignores that it is fully qualified and goes on to resolve the generic itself when instead it should just check if the types are compatible
Yeah, that's what I was thinking
By the way, up until now I just brought fixes like that to ooc-lang/rock, would you like me to open PRs on magic-lang too?
If you want, but really, it is our responsibility to monitor ooc-lang/rock
for bug fixes etc. I've been so busy with testing the bounty PRs and some other stuff that i've completely neglected to do that lately. It would of course be greatly appreciated if you did! :-)
(There is already one bug fix that went through on ooc and not magic, #983)
Thanks for the heads-up, I will check that out!
Ok, I'll make sure I mirror at least major bug fixes in both repositories to make your life easier, since it's only a matter of making a new branch and applying a diff anyways :)
@shamanas
I see that you are explicitly disallowing extensions for known types such as: extend Foo<Int>
. Is there no way of allowing both ways (as requested in the bounty)? In my head, rock would consolidate all the extensions and generate some sort of error upon redefinitions/collisions. How hard would this be to implement?
@thomasfanell
Hmm..
I think it's a natural limitation of generics.
What you are asking for is really easy to do with templates.
However, generics are meant to be values where you do not actually care about their types, so extending a realized generic type feels weird (at least to me) and would probably require a pretty huge amount of hacking in rock to achieve.
Basically, this can be done by having a list of type realized addons (extensions) in TypeDecl, then finding the best addon with some scoring and resolving calls from it.
However, the methods you are adding could still not handle generic values as their real type.
Anyways, the answer is that it's possible but using some pretty ugly hacks and the result may not be what you expect when you are writing the extention's methods.
@shamanas Yeah, I agree that it feels weird. I just wanted to see if you had any input on the matter. Let's call this one a done deal. :tada:
@fredrikbryntesson
As well as template classes and mixed (generic template) classes.
How the solution works:
We construct a mapping of the new generic names to the old definitions.
Then, we forward accesses to them to the original definitions.
We also keep illegal names, old names that don't appear in the extended generic names so we can give accurate error messages if they are accessed.
In addition to that, we need to change the way unrolling of properties is done inside the addon, since the addon never used to be in the trail.