Skip to content

Fix #6286: Fix ElimOpaque transformation #6298

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 5 commits into from
Apr 13, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
ref(defn.DottyArraysModule).select(defn.newArrayMethod).withSpan(span)

if (!ctx.erasedTypes) {
assert(!TypeErasure.isGeneric(elemTpe)) //needs to be done during typer. See Applications.convertNewGenericArray
assert(!TypeErasure.isGeneric(elemTpe), elemTpe) //needs to be done during typer. See Applications.convertNewGenericArray
newArr.appliedToTypeTrees(TypeTree(returnTpe) :: Nil).appliedToArgs(clsOf(elemTpe) :: clsOf(returnTpe) :: dims :: Nil).withSpan(span)
} else // after erasure
newArr.appliedToArgs(clsOf(elemTpe) :: clsOf(returnTpe) :: dims :: Nil).withSpan(span)
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/core/DenotTransformers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ object DenotTransformers {
/** A transformer that only transforms SymDenotations */
trait SymTransformer extends DenotTransformer {

/** Tramsform the info of a denotation that is not a Symdenotation */
def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = tp
Copy link
Member

Choose a reason for hiding this comment

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

I would call it transformNonSymInfo to avoid confusion. Or perhaps just make ElimOpaque a regular DenotTransformer, we can always add another kind of Transformer later if we find more uses for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK for transformNonSymInfo. I think it's better to leave it there to make it clear that something has to be done if a SymTranformer also changes the infos of symbols.


def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation

def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = ref match {
case ref: SymDenotation => transformSym(ref)
case _ => ref
case _ => ref.derivedSingleDenotation(ref.symbol, transformInfo(ref.info, ref.symbol))
Copy link
Member

Choose a reason for hiding this comment

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

Isn't derivedSingleDenotation going to force the info of the ref?

Copy link
Member

Choose a reason for hiding this comment

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

Ah in fact ref.info on that line does that more directly :). That means we're suddenly forcing all infos of non-sym SingleDenotations which doesn't sound ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Let's make it a DenotTransformer instead.

}
}

Expand Down
11 changes: 3 additions & 8 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import reporting.diagnostic.Message
import reporting.diagnostic.messages.BadSymbolicReference
import reporting.trace
import collection.mutable
import transform.TypeUtils._

import scala.annotation.internal.sharable

Expand Down Expand Up @@ -1082,16 +1083,10 @@ object SymDenotations {
* containing object.
*/
def opaqueAlias(implicit ctx: Context): Type = {
if (isOpaqueHelper) {
if (isOpaqueHelper)
owner.asClass.classInfo.selfType match {
case RefinedType(_, _, TypeBounds(lo, _)) =>
def extractAlias(tp: Type): Type = tp match {
case OrType(alias, _) => alias
case tp: HKTypeLambda => tp.derivedLambdaType(resType = extractAlias(tp.resType))
}
extractAlias(lo)
case RefinedType(_, _, bounds) => bounds.extractOpaqueAlias
}
}
else NoType
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,13 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
protected def typeApplyText[T >: Untyped](tree: TypeApply[T]): Text = {
val isQuote = tree.fun.hasType && tree.fun.symbol == defn.InternalQuoted_typeQuote
val (open, close) = if (isQuote) (keywordStr("'["), keywordStr("]")) else ("[", "]")
toTextLocal(tree.fun).provided(!isQuote) ~ open ~ toTextGlobal(tree.args, ", ") ~ close
val funText = toTextLocal(tree.fun).provided(!isQuote)
tree.fun match {
case Select(New(tpt), nme.CONSTRUCTOR) if tpt.typeOpt.dealias.isInstanceOf[AppliedType] =>
funText // type was already printed by toText(new)
case _ =>
funText ~ open ~ toTextGlobal(tree.args, ", ") ~ close
}
}

protected def toTextCore[T >: Untyped](tree: Tree[T]): Text = {
Expand Down
15 changes: 4 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/ElimOpaque.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,15 @@ package dotty.tools.dotc
package transform

import core._
import Names._
import dotty.tools.dotc.transform.MegaPhase._
import ast.Trees._
import ast.untpd
import Flags._
import Types._
import Constants.Constant
import Contexts.Context
import Symbols._
import Decorators._
import Annotations._
import Annotations.ConcreteAnnotation
import Denotations.SingleDenotation
import SymDenotations.SymDenotation
import scala.collection.mutable
import DenotTransformers._
import NameOps._
import NameKinds.OuterSelectName
import StdNames._
import TypeUtils._

object ElimOpaque {
val name: String = "elimOpaque"
Expand All @@ -37,6 +27,9 @@ class ElimOpaque extends MiniPhase with SymTransformer {
// base types of opaque aliases change
override def changesBaseTypes = true

override def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type =
if (sym.isOpaqueHelper) TypeAlias(tp.extractOpaqueAlias) else tp

def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation =
if (sym.isOpaqueHelper) {
sym.copySymDenotation(
Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/TypeUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,17 @@ object TypeUtils {
/** The `*:` equivalent of an instance of a Tuple class */
def toNestedPairs(implicit ctx: Context): Type =
TypeOps.nestedPairs(tupleElementTypes)

/** Extract opaque alias from TypeBounds type that combines it with the reference
* to the opaque type itself
*/
def extractOpaqueAlias(implicit ctx: Context): Type = self match {
case TypeBounds(lo, _) =>
def extractAlias(tp: Type): Type = tp match {
case OrType(alias, _) => alias
case self: HKTypeLambda => self.derivedLambdaType(resType = extractAlias(self.resType))
}
extractAlias(lo)
}
}
}
6 changes: 6 additions & 0 deletions tests/pos/i6286.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
object A {
opaque type T33 = Int
object T33 {
val a = new Array[T33](3)
}
}