Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic inheritance is broken #2665

Open
asterite opened this issue May 27, 2016 · 18 comments
Open

Generic inheritance is broken #2665

asterite opened this issue May 27, 2016 · 18 comments

Comments

@asterite
Copy link
Member

Right now inheriting from generic types, and even inheriting a non-generic type that's not Reference is broken. There are many issues related to this, which I'll link here:

And one pending decision:

All of these should be fixed. The main issue is that the current implementation is very weak and needs a redesign. This is only a matter of time.

This issue will be used to mark all bugs related to generics as a duplicate of this one, because it's essentially one problem.

@asterite
Copy link
Member Author

asterite commented Sep 1, 2016

I'll keep this issue open for now, until we test it a bit more, but all or most of the bugs above are now fixed on master.

@benoist
Copy link
Contributor

benoist commented Jan 11, 2017

I'm not really sure about this example, should this work or not?

https://play.crystal-lang.org/#/r/1k1i

@spalladino
Copy link
Contributor

@benoist what seems to be the problem in that example?

@benoist
Copy link
Contributor

benoist commented Jan 11, 2017

Ah sorry I should have explained it a bit better. I'd expected that adding 2 Int32Arrays it would also return an Int32Array. This is currently not the case

@spalladino
Copy link
Contributor

Well, Array#+ is documented to return an Array, and not an instance of the same class that received the + message. As such, I'd not expect Int32Array to be returned in that example.

Anyhow, if you do believe it should, or have a similar issue, please open a new issue here; the idea of this one is to act as an "index" of generic-related stuff.

@benoist
Copy link
Contributor

benoist commented Jan 13, 2017

No I think I agree with you. If I want it to return a concatenation of self I should explicitly do so.
Thinking about it, inheriting from a generic is probably not a good idea anyway, composition is probably better.

@spalladino
Copy link
Contributor

That's what I was going to suggest: either composition, or just declaring an alias if what you want is to have a friendlier name for a particular instantiation of a generic.

@ezrast
Copy link

ezrast commented Feb 2, 2017

Would further bug reports regarding generic inheritance be useful, or just noise at this point?

@RX14
Copy link
Contributor

RX14 commented Feb 2, 2017

@ezrast Bug reports are always welcome

@Sija
Copy link
Contributor

Sija commented Nov 14, 2017

Could we strike through in description issues/prs which were closed/merged? /cc @asterite

@RX14
Copy link
Contributor

RX14 commented Nov 17, 2017

This issue should be closed and made a project or label.

@miketheman
Copy link
Contributor

#codetriage
This came up today.
It appears that all related issues pointed out here have been closed and released, so I'm recommending closing this issue.

@ezrast
Copy link

ezrast commented Sep 19, 2018

@miketheman, The kernel of the issue, that "the current implementation is very weak and needs a redesign", has not yet been addressed to my knowledge. Several related bugs remain in latest Crystal, such as #4059 (which was closed as a duplicate of this issue), #5290, and this:

module Base(T); end
module Derived; extend Base(Int32); end
arr = [Derived] of Base(Int32)
pp arr.first == arr.first # false
pp Derived == arr.first   # false
pp arr.first == Derived   # true

@RX14
Copy link
Contributor

RX14 commented Sep 20, 2018

@ezrast please open as a seperate issue, that code shouldn't compile as Derived is Derived.class and Base(Int32).class. Derived.new wouldn't compile either, if it was a class and had a ctor that code would compile fine.

@miketheman
Copy link
Contributor

Considering there's a new issue to track the Derived status, I think this issue could be closed.

@Sija
Copy link
Contributor

Sija commented Sep 21, 2018

@miketheman As far as I can see lot of issues have been closed (without resolution) in favor of this one, so it's kinda recursive situation. Closing this will leave 'em forgotten for the future.

@HertzDevil
Copy link
Contributor

I reviewed every linked issue after #2665 (comment) and it seems the only ones that still break on master are #4086, #6760, and #10133. So once these are fixed I think we should close this meta-issue and rely on the kind:bug + topic:compiler:generics tags solely (or projects, but it seems nobody maintains them).

@straight-shoota
Copy link
Member

I have re-opened those individual issues. It's probably easier to target them directly in upcoming PRs (in case a PR spans multiple, that works as well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants