Skip to content

Remove incorrect type simplification #9019

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

Merged
merged 1 commit into from
May 21, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented May 21, 2020

TypeOps#simplify is supposed to produce a type =:= the original
type, but it replaced type variables created using newTypeVar by one
of their bound, this is wrong since that type variable might get
instantiated to a different type later. This simplification was added
in #2209 to prevent a pickling crash in tests/pos/i2152.scala but does
not appear to be needed anymore: an uninstantiated type variable being
preserved after typer is always wrong, so the underlying bug must have
been fixed since then.

`TypeOps#simplify` is supposed to produce a type `=:=` the original
type, but it replaced type variables created using `newTypeVar` by one
of their bound, this is wrong since that type variable might get
instantiated to a different type later. This simplification was added
in scala#2209 to prevent a pickling crash in tests/pos/i2152.scala but does
not appear to be needed anymore: an uninstantiated type variable being
preserved after typer is always wrong, so the underlying bug must have
been fixed since then.
@smarter smarter requested a review from odersky May 21, 2020 16:03
@odersky
Copy link
Contributor

odersky commented May 21, 2020

test performance please

@odersky
Copy link
Contributor

odersky commented May 21, 2020

let's just check that this has no performance implications. Otherwise LGTM

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/9019/ to see the changes.

Benchmarks is based on merging with master (38272aa)

@smarter smarter merged commit da0e938 into scala:master May 21, 2020
@smarter smarter deleted the fix-simplification branch May 21, 2020 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants