Skip to content

Change variances in TypeMaps and Accumulators #2934

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.

Sign up for GitHub

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 3 commits into from
Aug 2, 2017
Merged

Change variances in TypeMaps and Accumulators #2934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
805fdd4
Refine variance for NamedTypes in TypeMaps
odersky Jul 29, 2017
60c7cac
Treat variances in type accumulators and type maps the same
odersky Jul 29, 2017
8683f66
Refine determination of prefix in typedIdent
odersky Jul 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3713,8 +3713,19 @@ object Types {
tp match {
case tp: NamedType =>
if (stopAtStatic && tp.symbol.isStatic) tp
else derivedSelect(tp, this(tp.prefix))

else {
val saved = variance
variance = variance max 0
// A prefix is never contravariant. Even if say `p.A` is used in a contravariant
// context, we cannot assume contravariance for `p` because `p`'s lower
// bound might not have a binding for `A` (e.g. the lower bound could be `Nothing`).
// By contrast, covariance does translate to the prefix, since we have that
// if `p <: q` then `p.A <: q.A`, and well-formedness requires that `A` is a member
// of `p`'s upper bound.
val prefix1 = this(tp.prefix)
variance = saved
derivedSelect(tp, prefix1)
}
case _: ThisType
| _: BoundType
| NoPrefix => tp
Expand Down Expand Up @@ -3913,9 +3924,9 @@ object Types {

protected var variance = 1

protected def applyToPrefix(x: T, tp: NamedType) = {
protected final def applyToPrefix(x: T, tp: NamedType) = {
val saved = variance
variance = 0
variance = variance max 0 // see remark on NamedType case in TypeMap
val result = this(x, tp.prefix)
variance = saved
result
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Inferencing.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,6 @@ object Inferencing {
case _ =>
foldOver(vmap, t)
}
override def applyToPrefix(vmap: VarianceMap, t: NamedType) =
apply(vmap, t.prefix)
}

/** Include in `vmap` type variables occurring in the constraints of type variables
Expand Down
15 changes: 14 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,20 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
if (qualifies(defDenot)) {
val found =
if (isSelfDenot(defDenot)) curOwner.enclosingClass.thisType
else curOwner.thisType.select(name, defDenot)
else {
val effectiveOwner =
if (curOwner.isTerm && defDenot.symbol.isType)
// Don't mix NoPrefix and thisType prefixes, since type comparer
// would not detect types to be compatible. Note: If we replace the
// 2nd condition by `defDenot.symbol.maybeOwner.isType` we get lots
// of failures in the `tastyBootstrap` test. Trying to compile these
// files in isolation works though.
Copy link
Member

Choose a reason for hiding this comment

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

testPickling also failed, here's a simplified way to reproduce the failure:

./bin/dotc -Xprint-types -Ytest-pickler compiler/src/dotty/tools/io/File.scala compiler/src/dotty/tools/io/Jar.scala

This fails with a pickling difference, here's the diff:

--- before-pickling.txt 2017-07-30 16:10:33.955539823 +0200
+++ after-pickling.txt  2017-07-30 16:10:33.955539823 +0200
@@ -111,7 +111,7 @@
           <<new java.util.jar.JarFile:java.util.jar.JarFile>:
             ((x$0: java.io.File): java.util.jar.JarFile)
           >
-        (<<Jar.this.file:dotty.tools.io.File>.jfile:=> dotty.tools.io.JFile>):
+        (<<Jar.this.file:dotty.tools.io.File>.jfile:<notype>>):
           java.util.jar.JarFile
         >
       lazy val manifest: scala.Option[java.util.jar.Manifest] =

// TODO: Investigate why that happens.
defDenot.symbol.owner
else
curOwner
effectiveOwner.thisType.select(name, defDenot)
}
if (!(curOwner is Package) || isDefinedInCurrentUnit(defDenot))
result = checkNewOrShadowed(found, definition) // no need to go further out, we found highest prec entry
else {
Expand Down