Skip to content

Commit

Permalink
Change comparisons of opaque types.
Browse files Browse the repository at this point in the history
Previously we had for any type `T` in
```
object m {
  type T
}
```
that `m.this.T =:= m.T` as long as we are inside object `m` (outside, `m.this.T` makes no sense).
Now, assume `T` is an opaque type.
```
object m {
  opaque type T = Int
}
```
Inside `m` we have `this.m.T =:= Int`. Is it also true inside `m` that `m.T =:= Int`?
Previously, we said no, which means that subtyping and =:= equality were not transitive
in this case. We now say yes. We achieve this by lifting the external reference `m` to
`m.this` if we are inside object `m`.

An example that shows the difference is pos//opaque-groups-params.scala compiled from Tasty.
  • Loading branch information
odersky committed Jun 6, 2019
1 parent 6b3b446 commit 90b580b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 45 deletions.
87 changes: 54 additions & 33 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,13 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
implicit val ctx: Context = this.ctx
tp2.info match {
case info2: TypeAlias =>
recur(tp1, info2.alias) || tryPackagePrefix2(tp1, tp2)
recur(tp1, info2.alias)
case _ => tp1 match {
case tp1: NamedType =>
tp1.info match {
case info1: TypeAlias =>
if (recur(info1.alias, tp2)) return true
if (tp1.prefix.isStable) return tryPackagePrefix1(tp1, tp2)
if (tp1.prefix.isStable) return false
// If tp1.prefix is stable, the alias does contain all information about the original ref, so
// there's no need to try something else. (This is important for performance).
// To see why we cannot in general stop here, consider:
Expand All @@ -261,7 +261,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
if ((sym1 ne NoSymbol) && (sym1 eq sym2))
ctx.erasedTypes ||
sym1.isStaticOwner ||
isSubType(stripPackageObject(tp1.prefix), stripPackageObject(tp2.prefix)) ||
isSubType(tp1.prefix, tp2.prefix) ||
thirdTryNamed(tp2)
else
( (tp1.name eq tp2.name)
Expand Down Expand Up @@ -360,7 +360,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
tp1.info match {
case info1: TypeAlias =>
if (recur(info1.alias, tp2)) return true
if (tp1.prefix.isStable) return tryPackagePrefix1(tp1, tp2)
if (tp1.prefix.isStable) return tryLiftedToThis1
case _ =>
if (tp1 eq NothingType) return true
}
Expand Down Expand Up @@ -463,7 +463,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
narrowGADTBounds(tp2, tp1, approx, isUpper = false)) &&
{ tp1.isRef(NothingClass) || GADTusage(tp2.symbol) }
}
isSubApproxHi(tp1, info2.lo) || compareGADT || fourthTry
isSubApproxHi(tp1, info2.lo) || compareGADT || tryLiftedToThis2 || fourthTry

case _ =>
val cls2 = tp2.symbol
Expand Down Expand Up @@ -722,7 +722,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
narrowGADTBounds(tp1, tp2, approx, isUpper = true)) &&
{ tp2.isRef(AnyClass) || GADTusage(tp1.symbol) }
}
isSubType(hi1, tp2, approx.addLow) || compareGADT
isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1
case _ =>
def isNullable(tp: Type): Boolean = tp.widenDealias match {
case tp: TypeRef => tp.symbol.isNullableClass
Expand Down Expand Up @@ -976,7 +976,8 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
case _ =>
fourthTry
}
}
} || tryLiftedToThis2

case _: TypeVar =>
recur(tp1, tp2.superType)
case tycon2: AnnotatedType if !tycon2.isRefining =>
Expand All @@ -1003,9 +1004,11 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
isSubType(bounds(param1).hi.applyIfParameterized(args1), tp2, approx.addLow)
case tycon1: TypeRef =>
val sym = tycon1.symbol
!sym.isClass && (
!sym.isClass && {
defn.isCompiletime_S(sym) && compareS(tp1, tp2, fromBelow = false) ||
recur(tp1.superType, tp2))
recur(tp1.superType, tp2) ||
tryLiftedToThis1
}
case tycon1: TypeProxy =>
recur(tp1.superType, tp2)
case _ =>
Expand Down Expand Up @@ -1046,6 +1049,16 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
def isSubApproxHi(tp1: Type, tp2: Type): Boolean =
tp1.eq(tp2) || tp2.ne(NothingType) && isSubType(tp1, tp2, approx.addHigh)

def tryLiftedToThis1: Boolean = {
val tp1a = liftToThis(tp1)
(tp1a ne tp1) && recur(tp1a, tp2)
}

def tryLiftedToThis2: Boolean = {
val tp2a = liftToThis(tp2)
(tp2a ne tp2) && recur(tp1, tp2a)
}

// begin recur
if (tp2 eq NoType) false
else if (tp1 eq tp2) true
Expand Down Expand Up @@ -1075,31 +1088,39 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
}
}

/** If `tp` is a reference to a package object, a reference to the package itself,
* otherwise `tp`.
*/
private def stripPackageObject(tp: Type) = tp match {
case tp: TermRef if tp.symbol.isPackageObject => tp.symbol.owner.thisType
case tp: ThisType if tp.cls.isPackageObject => tp.cls.owner.thisType
case _ => tp
}

/** If prefix of `tp1` is a reference to a package object, retry with
* the prefix pointing to the package itself, otherwise `false`
*/
private def tryPackagePrefix1(tp1: NamedType, tp2: Type) = {
val pre1 = tp1.prefix
val pre1a = stripPackageObject(pre1)
(pre1a ne pre1) && isSubType(tp1.withPrefix(pre1a), tp2)
}

/** If prefix of `tp2` is a reference to a package object, retry with
* the prefix pointing to the package itself, otherwise `false`
/** If `tp` is an external reference to an enclosing module M that contains opaque types,
* convert to M.this.
* Note: It would be legal to do the lifting also if M does not contain opaque types,
* but in this case the retries in tryLiftedToThis would be redundant.
*/
private def tryPackagePrefix2(tp1: Type, tp2: NamedType) = {
val pre2 = tp2.prefix
val pre2a = stripPackageObject(pre2)
(pre2a ne pre2) && isSubType(tp1, tp2.withPrefix(pre2a))
private def liftToThis(tp: Type): Type = {

def findEnclosingThis(moduleClass: Symbol, from: Symbol): Type =
if ((from.owner eq moduleClass) && from.isPackageObject && from.is(Opaque)) from.thisType
else if (from.is(Package)) tp
else if ((from eq moduleClass) && from.is(Opaque)) from.thisType
else if (from eq NoSymbol) tp
else findEnclosingThis(moduleClass, from.owner)

tp.stripTypeVar.stripAnnots match {
case tp: TermRef if tp.symbol.is(Module) =>
findEnclosingThis(tp.symbol.moduleClass, ctx.owner)
case tp: TypeRef =>
val pre1 = liftToThis(tp.prefix)
if (pre1 ne tp.prefix) tp.withPrefix(pre1) else tp
case tp: ThisType if tp.cls.is(Package) =>
findEnclosingThis(tp.cls, ctx.owner)
case tp: AppliedType =>
val tycon1 = liftToThis(tp.tycon)
if (tycon1 ne tp.tycon) tp.derivedAppliedType(tycon1, tp.args) else tp
case tp: TypeVar if tp.isInstantiated =>
liftToThis(tp.inst)
case tp: AnnotatedType =>
val parent1 = liftToThis(tp.parent)
if (parent1 ne tp.parent) tp.derivedAnnotatedType(parent1, tp.annot) else tp
case _ =>
tp
}
}

/** Optionally, the `n` such that `tp <:< ConstantType(Constant(n: Int))` */
Expand Down
23 changes: 14 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,18 @@ class Typer extends Namer

val curOwner = ctx.owner

/** Is curOwner a package object that should be skipped?
* A package object should always be skipped if we look for a term.
* That way we make sure we consider all overloaded alternatives of
* a definition, even if they are in different source files.
* If we are looking for a type, a package object should ne skipped
* only if it does not contain opaque definitions. Package objects
* with opaque definitions are significant, since opaque aliases
* are only seen if the prefix is the this-type of the package object.
*/
def isTransparentPackageObject =
curOwner.isPackageObject && (name.isTermName || !curOwner.is(Opaque))

// Can this scope contain new definitions? This is usually the first
// context where either the scope or the owner changes wrt the
// context immediately nested in it. But for package contexts, it's
Expand All @@ -279,14 +291,7 @@ class Typer extends Namer
val isNewDefScope =
if (curOwner.is(Package) && !curOwner.isRoot) curOwner ne ctx.outer.owner
else ((ctx.scope ne lastCtx.scope) || (curOwner ne lastCtx.owner)) &&
(name.isTypeName || !curOwner.isPackageObject)
// If we are looking for a term, skip package objects and wait until we
// hit the enclosing package. That way we make sure we consider
// all overloaded alternatives of a definition, even if they are
// in different source files.
// On the other hand, for a type we should stop at the package object
// since the type might be opaque, so we need to have the package object's
// thisType as prefix in order to see the alias.
!isTransparentPackageObject

if (isNewDefScope) {
val defDenot = ctx.denotNamed(name, required)
Expand Down Expand Up @@ -1663,7 +1668,7 @@ class Typer extends Namer
val parentsWithClass = ensureFirstTreeIsClass(parents.mapconserve(typedParent).filterConserve(!_.isEmpty), cdef.nameSpan)
val parents1 = ensureConstrCall(cls, parentsWithClass)(superCtx)

var self1 = typed(self)(ctx.outer).asInstanceOf[ValDef] // outer context where class members are not visible
val self1 = typed(self)(ctx.outer).asInstanceOf[ValDef] // outer context where class members are not visible
if (self1.tpt.tpe.isError || classExistsOnSelf(cls.unforcedDecls, self1)) {
// fail fast to avoid typing the body with an error type
cdef.withType(UnspecifiedErrorType)
Expand Down
6 changes: 3 additions & 3 deletions tests/run/implied-priority.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,13 @@ def test4 = {
*
* It employs a more re-usable version of the result refinement trick.
*/
opaque type HigherPriority = Any
object HigherPriority {
def inject[T](x: T): T & HigherPriority = x
opaque type Type = Any
def inject[T](x: T): T & Type = x
}

object fallback5 {
implied [T] for (E[T] & HigherPriority) given (ev: E[T] = new E[T]("fallback")) = HigherPriority.inject(ev)
implied [T] for (E[T] & HigherPriority.Type) given (ev: E[T] = new E[T]("fallback")) = HigherPriority.inject(ev)
}

def test5 = {
Expand Down

0 comments on commit 90b580b

Please sign in to comment.