Change hk take 2 #802
Merged
Change hk take 2 #802
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avoid forcing info if the reference goes to a class. This avoided a CyclicReference when reading Scala's standard library form pos/collections when fiddling with the hk logic.
In Namer, eta expand any type argument that corresponds to a higher-kinded type parameter. Also, check that all type parameter lists are fully applied.
With the hk-types schem changed, we need to make sure that actual and formal argument lists of parameterized types have the same length.
It used to be "assertion error: NoType" whenever sigName git a projection of a missing member. Now we find out about what type was projected.
Now catches attempts to recursively force a LazyRef type that's in train of being evaluated.
Erreneous programs could have a difference in lengths between type parameters and type args, but this is tested anyway in Typer.
... and move to TypeApplications. isLambda test was the wrong way before.
Without the additional `typeParams.nonEmpty` condition we got a crash in t1439.scala
Previously, we did this only in applications in rhs of type definitions. Need to do it everywhere.
This would lead to a crash. Example is in Predef:
object Pair
type Pair
EtaReduce will be used to keep applications on eta expanded methods small.
parameterizeWith picked between simple hk types and lambda abstraction. No longer needed because now we always lambda abstract.
Needed to avoid cycles involving F-boundes hk-types when reading Scala2 collection classes with new hk-scheme.
When unpickling from Scala2 TypeRefs with arguments which do not refer to classes, use EtaExpand instead of LambdaAbstract. Lambda Abstrct is wrong since it drops type arguments.
New hk-scheme caused cycles in elimExistentials which are fixed by this patch.
Derived types already contain the lambda abstractoion; lambda abstracting them again would cause a double lambda.
Discrepancies between numbers of formal and actual type arguments were observed when typing partialFunctions.scala under new scheme. Should come back to this when subtyping is rewrittem/simplified to work with new hk-scheme. Maybe testLifted is no longer needed at all.
This aligns typeParams and rawTypeParams. It's not strictly to necessary, though.
Rewrite a type application like
([HK$0] => C[HK$0])(T) to C[T]
Avoids application cahins to become unnecessarly large.
Used to be just instantiated lambdas. With the new scheme every type with a kind higher than * needs to be projected with #Apply.
Turn a possible NPE into an AssertionError. The latter are caught in pickleTree, so an error leaves a trace about what was pickled.
Previously, only pattern bound arguments were adapated. This was an oversight. Also, change logix so that we survive empty type parameter lists. This was also an oversight before.
Want to have a unique name for Apply, so that tests for higher-kinded types become cheaper.
by removing dead case.
These are not user-accessible types, so no need to follow type convention and write in upper case. Also, rename occurrences of lambda to Lambda, to make clear we mean a type lambda.
It's no longer needed with new hk scheme.
Replace occurrences of isLambda with isHK, because isHK is a bit faster and simplier.
Accidentally forwarded to rawTypeParams. This solved the problem with mismatching type params in appliedTo that was caught in testLifted.
Now also allows to reduce something like ([T] => Map[T, String]) to Map[_, String]
...unless the HK type can be eta-reduced to a class type.
1) Lambda abstract now records bounds of abstracted type parameters in TypeLambda 2) Eta-reduce likewise keeps the bounds it finds in the TypeLambda 3) Eta-reduce now also translates hk$i references to type parameters of the reduced type.
The original IterableSelfRec is not syntactically legal after the hk changes. I attempted to fix, but there's still a type error. Need to investigate whether this is a true error or a bug.
It turns out that asSeenFrom can produce types that get projected with $apply but that are not higher-kinded. An exampple failure is in Iter3, andother in scala.collection.immutable.Map (which is now part of the test suite). We now detect that situation, and eta expand the projected type in `derivedSelect`, this will force a subssequent `lookupRefined` which will give the desired normalized type. Also added is a configurable test that checks that $apply projected tyeps are in fact higher-kinded.
odersky
force-pushed
the
change-hk-1
branch
from
e9ca3b0 to
71e3133
Compare
Closed
Fixes suggested by @marter when reviewing previous hk PR.
|
/rebuild |
|
Seems the tests are not running? |
|
/rebuild |
|
I'll look into why Scabot isn't kicking in here. |
|
it was because the PR includes more than 30 commits, which exceeded the GitHub API's default page size. I fixed Scabot to request a page size of 100 commits (the maximum allowed), and manually started a new Jenkins run: https://scala-ci.typesafe.com/view/dotty/job/dotty-master-validate-main/437/ |
|
So what happens when we have a PR with 101 commits? :) |
I dare you! |
|
Otherwise LGTM, though I do not fully understand all the consequences of this change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebased #731 to master and added some changes to make all tests pass, including the standard collection tests in #787. In retrospect, it would have been good to add this ealier because the fixes
of #779 in #787 were cruder than what is in this PR. In particular, we no longer need to intercept cyclic reference errors.
Review by @DarkDimius or @smarter