Skip to content

Refactor lambda types #2121

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 35 commits into from
Apr 6, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 19, 2017

This is a huge refactoring which tries to exploit commonalities between PolyTypes and MethodTypes on the one hand and PolyParams (now called TypeParamRef) and MethodParams (now called TermParamRef) on the other hand. It creates a 2x2 matrix of lambda types where one dimension is its kind (* vs higher kinded) and the other is whether the abstraction is over terms or types.

One square of the matrix is currently unpopulated: higher-kinded types over term parameters do not exit yet. But given the refactorings they would be easy to add.

There are several benefits achieved by exploiting symmetries:

  • Reduce code duplication
  • Detect and fix subtle bugs about parameter-dependent methods which broke the symmetry before
  • Prepare the way for term-dependent lambdas on the type level
  • Better distinguish PolyTypes (on the * level) from higher kinded type lambdas. In particular, hk type lambdas are naturally type proxies but it's dangerous to treat polytypes as type proxies.

Based on #2103

@odersky
Copy link
Contributor Author

odersky commented Mar 20, 2017

Here's a high-level summary of what was done:

Splits: PolyType becomes HKTypeLambda or PolyType.

Common base traits:

LambdaType   |   TermLambda      |   TypeLambda
-------------+-------------------+------------------
HKLambda     |  (HKTermLambda)   |   HKTypeLambda
MethodOrPoly |   MethodType	 |   PolyType

Renamings:

from             to
--------------------------------
PolyTypeTree     LambdaTypeTree
PolyParam        TypeParamRef
MethodParam      TermParamRef
TypeParamInfo    ParamInfo
paramBounds      paramInfo
paramType        paramInfo

@felixmulder felixmulder requested a review from smarter April 4, 2017 09:58
odersky added a commit to dotty-staging/dotty that referenced this pull request Apr 4, 2017
This commit can hopefully be reverted once scala#2121 is in.
@odersky odersky force-pushed the change-merge-method-poly branch from ad6cb52 to 4939b47 Compare April 6, 2017 11:14
@odersky
Copy link
Contributor Author

odersky commented Apr 6, 2017

Rebased to master

odersky added 25 commits April 6, 2017 13:15
This leads to a slight overall simplification, harmonizes pickle
format with internal representation, and makes MethodTypes and
PolyTypes more similar to each other.

I believe the change is useful as it is, but in particular it is
a useful step for an eventual unification of MethodTypes and
PolyTypes.
It was a red herring. Symbolic names are expanded anyway to $plus / $minus,
so they can't be confused with a variance prefix.
and generalize MethodParam to ParamRef, and
TypeParamInfo to ParamInfo
MethodTypes have paramTypes whereas PolyTypes have paramBounds.
We now harmonize by alling both paramInfos, and parameterizing
types that will become common to both.
Trying to bring PolyTypes closer to TypeLambdas
It seems we need a more refined way to deal with non-variant variables
in pattern matches. See branch change-patmat-undet for a WIP. For the
moment we disable -strict to be able to compile latest version of dotty.
It seems we need a more refined way to deal with non-variant variables
in pattern matches. See branch change-patmat-undet for a WIP. For the
moment we disable -strict to be able to compile latest version of dotty. (reverted from commit c8fe830)
Also, rename LambdaOver{Type,Term}s to {Type,Term}Lambda
... to distinguish between HK(proxy) and *(ground) types.
Also, refactor some more methods to keep it DRY.
Replace with ParamRef
Replace with LambdaType
- Use TypeLambda instead of PolyType.
- Further harmonize factory operations
Use fromParams instead.
odersky added 10 commits April 6, 2017 13:15
It's too surprising to leave it as a type proxy. In all circumstances except
erasure, it is not true that a PolyType is somehow the same as its result type.
Two benefits: (1) less code. (2) finding subtle bugs about
parameter dependent method types. By merging with PolyTypes
we are forced to take parameter dependencies into account.
 - split LambdaType.equals into two equals so that tests
   are more specific (also avoids type testing against
   a trait)
 - re-order cases in some pattern matches with the aim to
   (1) move common cases earlier, (2) move more expensive
   trait type tests later.
@odersky odersky merged commit 09cc237 into scala:master Apr 6, 2017
odersky added a commit to dotty-staging/dotty that referenced this pull request Jul 19, 2017
This commit can hopefully be reverted once scala#2121 is in. (reverted from commit 4c576bf)
odersky added a commit to dotty-staging/dotty that referenced this pull request Jul 19, 2017
This commit can hopefully be reverted once scala#2121 is in. (reverted from commit 4c576bf)
@allanrenucci allanrenucci deleted the change-merge-method-poly branch December 14, 2017 16:58
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.

1 participant