-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Revert: Don't automatically virtualize two types in the same hierarchy, unless one is deeper than the other #6351
Revert: Don't automatically virtualize two types in the same hierarchy, unless one is deeper than the other #6351
Conversation
…y, unless one is deeper than the other
fb4f380
to
f343019
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sad but, I guess necessary.
I guess it's acceptable. Given |
@ysbaddaden |
Sigh. Too bad then. |
My personal use case for not merging unions and go to the parent type was having an instance variable with a union type. For example a TypeDeclaration node can only have a Var, InstanceVar or ClassVar as targets, but these get condensed to ASTNode, so when dealing with this we have to do "else raise bug". I'm thinking whether we can make at least that case work, so when you explicitly define a type declaration like that it keeps it. It would be a special type of union that doesn't go to the parent type when merging it with other types. I'll have to play with this idea, not sure it can work or it's easy. But then, maybe it'll be too confusing, because sometimes it will get merged, sometimes not, and it will be hard to track. So initially I'd like this to get merged, and then we can think alternative solutions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert and try to explore alternatives for explicit unions.
Fixes #6349
Reverts 26635f1
To make it clear:
--release
when I compiled the compiler). EDIT: "Semantic (main)" takes about 1 second with a release compiler. And it's about 15K lines of code :-ONot unifying types in the same hierarchy was something that I, and probably others, wanted, but it's simply not possible. That's the whole reason why we unified types in the first place: to avoid this combinatorial explosion of union types.
And remember, this is not that terrible: we've been doing just fine before 0.25.0 without this "feature", and without me trying to play with this it would never have happened, so... :-)