Skip to content

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Apr 10, 2017

#2152 shows that dependent result type parameters can end up in
the types of terms, so we have to eliminate them. If we don't we
get orphan parameters in pickling.

Based on #2128. Only one commit is new.

@odersky odersky requested a review from smarter April 10, 2017 08:22
@propensive
Copy link
Contributor

I can confirm that this fixes the issue I described (in reduced form) in #2152.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be rebased, otherwise LGTM.

scala#2152 shows that dependent result type parameters can end up in
the types of terms, so we have to eliminate them. If we don't we
get orphan parameters in pickling.
Fix method name and comment
The original fix made run/hmap-covariant fail because a type variable
representing a dependent result parameter was instantiated. Trying something
else now.
@odersky
Copy link
Contributor Author

odersky commented Apr 12, 2017

@propensive Can you please check whether the latest commit also solves the original issue you were having? Unfortunately my original fix broke something else.

@odersky odersky merged commit 6b041ee into scala:master Apr 13, 2017
@allanrenucci allanrenucci deleted the fix-#2152 branch December 14, 2017 19:18
smarter added a commit to dotty-staging/dotty that referenced this pull request 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 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.
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