Bug fixes to make collection strawman compile #2331
Merged
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
We might get a wrong baseTypeRef cache when basetypes get populated while parent type infos of a class are not yet installed. So at the end of initializing a class type in namer, we should invalidate the caches of all its base types. Without the fix, BitSet in the collection strawman fails to compile.
Cycles can also arise for initialized type refs that appear in their own types.
That way we can trace where paramrefs come from.
Could create orphan paramrefs before.
We might need it in other places (e.g. self type comparisons). # Conflicts: # compiler/src/dotty/tools/dotc/typer/RefChecks.scala
We previously always failed for a situation like [X] => T <:< C unless the lhs was an eta-expansion.
A new setting, -Ysuppress-param-forwarding is used
to make forwarding type parameters confihurable.
Forwarding type parameters from base class to subclass instance
is done in `normalizeToClassRefs` for optimization, but I just found
out this optimization is not always type-preserving and in fact can
lead to type errors.
SortedMap.scala in the collection strawman is an example. With parameter
forwarding enabled, we get:
Error: immutable/SortedMap.scala --------------------------------------------
| with MapValuePolyTransforms[K, V, C] {
| ^
|Type argument C does not conform to upper bound [=X, +Y] =>
| strawman.collection.immutable.Map[=X, +Y] &
| strawman.collection.immutable.MapValuePolyTransforms[=X', +Y', LazyRef(C)]
|
|where: +Y is a type variable
| +Y' is a type variable
| =X is a type variable
| =X' is a type variable
If we turn off forwarding with the option, the strawman compiles OK.
Unfortunately turning the option off altogether does not work (yet).
It leads to increased compile times, and more tests fail with deep subtype
recursions. Dotty bootstrap seems to not terminate at all.
Related: Refine notion of self in RefChecks. This is
Needed to make pos/i1401.scala compile when forwardTypeParams is off.
# Conflicts:
# compiler/src/dotty/tools/dotc/typer/RefChecks.scala
We failed to take into account that a parameter might be forwarded to several types, in which case these types need to be conjoined. This led to a failure to compile the collection strawman unless forwarding was disabled.
It was a command line setting before. But since we do not need to selective suppress param forwarding for making things compile anymore, it's better not to expose it as such.
|
Comment copy pasted from #2240: I'd like to get this in soon, because I fear that if we wait, collections will get even more code that will fail in dotty. So we need to get to a point to do CI of the strawman with dotty and scalac. For this we need:
|
|
@DarkDimius I only posted a small comment about it: scala/collection-strawman#61 (comment) |
|
@DarkDimius The code pointed to by @julierf is what I showed you. |
smarter
force-pushed
the
fix-strawman-compile
branch
from
a3328cb to
1ebff50
Compare
smarter
approved these changes
May 1, 2017
| def forwardRef(argSym: Symbol, from: TypeName, to: TypeAlias) = argSym.info match { | ||
| case info @ TypeAlias(TypeRef(_: ThisType, `from`)) if info.variance == to.variance => | ||
| val existing = decls.lookup(argSym.name) | ||
| if (existing.exists) existing.info = existing.info & to |
There was a problem hiding this comment.
enterArgBindings sets the info using a LazyType, shouldn't the same thing be done here?
There was a problem hiding this comment.
It seems we get away with it. I have a tendency to wait for an actual CyclicReference popping up before we take that measure.
| * | ||
| * If multiple forwarders would be generated, join their `to` types with an `&`. | ||
| * | ||
| * @param cls The class for which parameter bindings should be forwarded |
There was a problem hiding this comment.
The params are not in the same order in the doc than in the declaration.
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.
Replaces #2240, and is based on #2330. Compared to #2240, this PR does not need tricky language changes. The changes to union types have already been subsumed in a simpler and more general
fashion in #2330. So all that remains here is bug fixes and better diagnostic capabilties.