Skip to content

Fix #3564: Allow annotated singleton types #3723

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 2 commits into from
Jan 4, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 6 additions & 5 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
followOuterLinks(This(tp.symbol.moduleClass.asClass))
else if (tp.symbol hasAnnotation defn.ScalaStaticAnnot)
Ident(tp)
else tp.prefix match {
case pre: SingletonType => followOuterLinks(singleton(pre)).select(tp)
case pre => Select(TypeTree(pre), tp)
} // no checks necessary
else {
val pre = tp.prefix
if (pre.isSingleton) followOuterLinks(singleton(pre.dealias)).select(tp)
else Select(TypeTree(pre), tp)
}

def ref(sym: Symbol)(implicit ctx: Context): Tree =
ref(NamedType(sym.owner.thisType, sym.name, sym.denot))
Expand Down Expand Up @@ -776,7 +777,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
applyOverloaded(tree, nme.EQ, that :: Nil, Nil, defn.BooleanType)

/** `tree.isInstanceOf[tp]`, with special treatment of singleton types */
def isInstance(tp: Type)(implicit ctx: Context): Tree = tp match {
def isInstance(tp: Type)(implicit ctx: Context): Tree = tp.dealias match {
case tp: SingletonType =>
if (tp.widen.derivesFrom(defn.ObjectClass))
tree.ensureConforms(defn.ObjectType).select(defn.Object_eq).appliedTo(singleton(tp))
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,10 @@ trait ConstraintHandling {
*/
def instanceType(param: TypeParamRef, fromBelow: Boolean): Type = {
def upperBound = constraint.fullUpperBound(param)
def isSingleton(tp: Type): Boolean = tp match {
def isMultiSingleton(tp: Type): Boolean = tp.stripAnnots match {
case tp: SingletonType => true
case AndType(tp1, tp2) => isSingleton(tp1) | isSingleton(tp2)
case OrType(tp1, tp2) => isSingleton(tp1) & isSingleton(tp2)
case AndType(tp1, tp2) => isMultiSingleton(tp1) | isMultiSingleton(tp2)
case OrType(tp1, tp2) => isMultiSingleton(tp1) & isMultiSingleton(tp2)
case _ => false
}
def isFullyDefined(tp: Type): Boolean = tp match {
Expand All @@ -274,7 +274,7 @@ trait ConstraintHandling {
// 1. If instance is from below and is a singleton type, yet upper bound is
// not a singleton type or a reference to `scala.Singleton`, widen the
// instance.
if (fromBelow && isSingleton(inst) && !isSingleton(upperBound)
if (fromBelow && isMultiSingleton(inst) && !isMultiSingleton(upperBound)
&& !upperBound.isRef(defn.SingletonClass))
inst = inst.widen

Expand Down
15 changes: 10 additions & 5 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ object Types {
case _: SingletonType | NoPrefix => true
case tp: RefinedOrRecType => tp.parent.isStable
case tp: ExprType => tp.resultType.isStable
case tp: AnnotatedType => tp.tpe.isStable
case _ => false
}

Expand Down Expand Up @@ -247,6 +248,9 @@ object Types {
case _ => NoType
}

/** Is this type a (possibly aliased) singleton type? */
def isSingleton(implicit ctx: Context) = dealias.isInstanceOf[SingletonType]

/** Is this type guaranteed not to have `null` as a value? */
final def isNotNull(implicit ctx: Context): Boolean = this match {
case tp: ConstantType => tp.value.value != null
Expand Down Expand Up @@ -354,7 +358,6 @@ object Types {
@tailrec final def typeSymbol(implicit ctx: Context): Symbol = this match {
case tp: TypeRef => tp.symbol
case tp: ClassInfo => tp.cls
// case ThisType(cls) => cls // needed?
case tp: SingletonType => NoSymbol
case tp: TypeProxy => tp.underlying.typeSymbol
case _ => NoSymbol
Expand Down Expand Up @@ -918,7 +921,7 @@ object Types {
/** Widen from singleton type to its underlying non-singleton
* base type by applying one or more `underlying` dereferences.
*/
final def widenSingleton(implicit ctx: Context): Type = stripTypeVar match {
final def widenSingleton(implicit ctx: Context): Type = stripTypeVar.stripAnnots match {
case tp: SingletonType if !tp.isOverloaded => tp.underlying.widenSingleton
case _ => this
}
Expand Down Expand Up @@ -3555,16 +3558,18 @@ object Types {
// ----- Annotated and Import types -----------------------------------------------

/** An annotated type tpe @ annot */
case class AnnotatedType(tpe: Type, annot: Annotation)
extends UncachedProxyType with ValueType {
case class AnnotatedType(tpe: Type, annot: Annotation) extends UncachedProxyType with ValueType {
// todo: cache them? but this makes only sense if annotations and trees are also cached.

override def underlying(implicit ctx: Context): Type = tpe

def derivedAnnotatedType(tpe: Type, annot: Annotation) =
if ((tpe eq this.tpe) && (annot eq this.annot)) this
else AnnotatedType(tpe, annot)

override def stripTypeVar(implicit ctx: Context): Type =
derivedAnnotatedType(tpe.stripTypeVar, annot)

override def stripAnnots(implicit ctx: Context): Type = tpe.stripAnnots
}

Expand Down Expand Up @@ -3965,7 +3970,7 @@ object Types {
*/
def tryWiden(tp: NamedType, pre: Type): Type = pre.member(tp.name) match {
case d: SingleDenotation =>
d.info match {
d.info.dealias match {
case TypeAlias(alias) =>
// if H#T = U, then for any x in L..H, x.T =:= U,
// hence we can replace with U under all variances
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,12 @@ object Erasure {
* in ExtensionMethods#transform.
*/
def cast(tree: Tree, pt: Type)(implicit ctx: Context): Tree = trace(i"cast ${tree.tpe.widen} --> $pt", show = true) {

def wrap(tycon: TypeRef) =
ref(u2evt(tycon.typeSymbol.asClass)).appliedTo(tree)
def unwrap(tycon: TypeRef) =
ref(evt2u(tycon.typeSymbol.asClass)).appliedTo(tree)


assert(!pt.isInstanceOf[SingletonType], pt)
if (pt isRef defn.UnitClass) unbox(tree, pt)
else (tree.tpe.widen, pt) match {
Expand Down
29 changes: 15 additions & 14 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -806,22 +806,23 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
def checkable(tree: Match): Boolean = {
// Possible to check everything, but be compatible with scalac by default
def isCheckable(tp: Type): Boolean =
!tp.hasAnnotation(defn.UncheckedAnnot) && (
ctx.settings.YcheckAllPatmat.value ||
tp.typeSymbol.is(Sealed) ||
tp.isInstanceOf[OrType] ||
(tp.isInstanceOf[AndType] && {
val and = tp.asInstanceOf[AndType]
isCheckable(and.tp1) || isCheckable(and.tp2)
}) ||
tp.isRef(defn.BooleanClass) ||
tp.typeSymbol.is(Enum) ||
canDecompose(tp) ||
(defn.isTupleType(tp) && tp.dealias.argInfos.exists(isCheckable(_)))
)
!tp.hasAnnotation(defn.UncheckedAnnot) && {
val tpw = tp.widen.dealias
ctx.settings.YcheckAllPatmat.value ||
tpw.typeSymbol.is(Sealed) ||
tpw.isInstanceOf[OrType] ||
(tpw.isInstanceOf[AndType] && {
val and = tpw.asInstanceOf[AndType]
isCheckable(and.tp1) || isCheckable(and.tp2)
}) ||
tpw.isRef(defn.BooleanClass) ||
tpw.typeSymbol.is(Enum) ||
canDecompose(tpw) ||
(defn.isTupleType(tpw) && tpw.argInfos.exists(isCheckable(_)))
}

val Match(sel, cases) = tree
val res = isCheckable(sel.tpe.widen.dealiasKeepAnnots)
val res = isCheckable(sel.tpe)
debug.println(s"checkable: ${sel.show} = $res")
res
}
Expand Down
7 changes: 2 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,7 @@ trait Checking {
*/
def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = vparamss match {
case (vparam :: Nil) :: _ if !(vparam.symbol is Implicit) =>
if (vparam.tpt.tpe.isInstanceOf[SingletonType])
ctx.error(s"implicit conversion may not have a parameter of singleton type", vparam.tpt.pos)
checkNotSingleton(vparam.tpt, " to be parameter type of an implicit conversion")
case _ =>
}

Expand Down Expand Up @@ -660,9 +659,7 @@ trait Checking {

/** Check that `tpt` does not refer to a singleton type */
def checkNotSingleton(tpt: Tree, where: String)(implicit ctx: Context): Tree =
if (tpt.tpe.isInstanceOf[SingletonType]) {
errorTree(tpt, ex"Singleton type ${tpt.tpe} is not allowed $where")
}
if (tpt.tpe.isSingleton) errorTree(tpt, ex"Singleton type ${tpt.tpe} is not allowed $where")
else tpt

/** Verify classes extending AnyVal meet the requirements */
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
case tp: MethodType =>
(tp.paramNames, tp.paramInfos, argss.head).zipped.foreach { (name, paramtp, arg) =>
def isByName = paramtp.dealias.isInstanceOf[ExprType]
paramBinding(name) = arg.tpe.stripAnnots.stripTypeVar match {
case argtpe: SingletonType if isIdempotentExpr(arg) => argtpe
paramBinding(name) = arg.tpe.dealias match {
case _: SingletonType if isIdempotentExpr(arg) => arg.tpe
case argtpe =>
val inlineFlag = if (paramtp.hasAnnotation(defn.InlineParamAnnot)) Inline else EmptyFlags
val (bindingFlags, bindingType) =
Expand Down Expand Up @@ -476,7 +476,7 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
}
case _: Ident =>
paramProxy.get(tree.tpe) match {
case Some(t: SingletonType) if tree.isTerm => singleton(t).withPos(tree.pos)
case Some(t) if tree.isTerm && t.isSingleton => singleton(t).withPos(tree.pos)
case Some(t) if tree.isType => TypeTree(t).withPos(tree.pos)
case None => tree
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ trait TypeAssigner {
def apply(tp: Type): Type = tp match {
case tp: TermRef
if toAvoid(tp.symbol) || partsToAvoid(mutable.Set.empty, tp.info).nonEmpty =>
tp.info.widenExpr match {
tp.info.widenExpr.dealias match {
case info: SingletonType => apply(info)
case info => range(tp.info.bottomType, apply(info))
}
Expand Down Expand Up @@ -124,7 +124,7 @@ trait TypeAssigner {
* 1. We first try a widening conversion to the type's info with
* the original prefix. Since the original prefix is known to
* be a subtype of the returned prefix, this can improve results.
* 2. IThen, if the approximation result is a singleton reference C#x.type, we
* 2. Then, if the approximation result is a singleton reference C#x.type, we
* replace by the widened type, which is usually more natural.
* 3. Finally, we need to handle the case where the prefix type does not have a member
* named `tp.name` anymmore. In that case, we need to fall back to Bot..Top.
Expand All @@ -133,7 +133,7 @@ trait TypeAssigner {
if (pre eq tp.prefix)
tp
else tryWiden(tp, tp.prefix).orElse {
if (tp.isTerm && variance > 0 && !pre.isInstanceOf[SingletonType])
if (tp.isTerm && variance > 0 && !pre.isSingleton)
apply(tp.info.widenExpr)
else if (upper(pre).member(tp.name).exists)
super.derivedSelect(tp, pre)
Expand Down Expand Up @@ -538,7 +538,7 @@ trait TypeAssigner {
tree.withType(sym.termRef)

def assignType(tree: untpd.Annotated, arg: Tree, annot: Tree)(implicit ctx: Context) =
tree.withType(AnnotatedType(arg.tpe.widen, Annotation(annot)))
tree.withType(AnnotatedType(arg.tpe, Annotation(annot)))

def assignType(tree: untpd.PackageDef, pid: Tree)(implicit ctx: Context) =
tree.withType(pid.symbol.termRef)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1593,7 +1593,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
if (ctx.mode is Mode.Type)
assignType(cpy.Annotated(tree)(arg1, annot1), arg1, annot1)
else {
val tpt = TypeTree(AnnotatedType(arg1.tpe.widen, Annotation(annot1)))
val tpt = TypeTree(AnnotatedType(arg1.tpe, Annotation(annot1)))
Copy link
Member

Choose a reason for hiding this comment

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

Actually thinking about this more, I think we should keep the .widen here.
I think we can only reach this case when the user write:

val x = 1: @unchecked

In this case I think it's fine to interpret this code as if the user had written:

val x = 1: Int @unchecked

If the user really wants to put the annotation on the singleton type, which is rather unusual, he can still explicitly write:

val x = 1: 1 @unchecked

This one will work fine because of the .widen which was dropped in TypeAssigner

Copy link
Contributor Author

@odersky odersky Jan 2, 2018

Choose a reason for hiding this comment

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

But that means #3564 fails, because we have precisely the case where we annotate a singleton with @uncheckedVariance.

Also, I would argue that someone writing

expr: @unchecked

is really annotating the expression expr, not its type. The type is an artifact of the way we express annotated expressions. So it is better that the annotation does not leak into inferred types of the lhs, say. See also #3427, which is another bug that would be fixed by this PR, but would not be fixed by the proposed change.

Copy link
Member

Choose a reason for hiding this comment

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

But that means #3564 fails, because we have precisely the case where we annotate a singleton with @uncheckedvariance.

I don't think so, in #3564 a singleton type is annotated, so ctx.mode is Type is true, this case is handled two lines above the current one, and only the .widen that was in TypeAssigner is relevant there.

So it is better that the annotation does not leak into inferred types of the lhs, say.

OK. I'm fine with this but note that this is a change of behavior with respect to Scala 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I changed the logic in Typer only, and #3564 started failing again.

assignType(cpy.Typed(tree)(arg1, tpt), tpt)
}
}
Expand Down Expand Up @@ -2268,7 +2268,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
// is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens.
err.typeMismatch(tree, pt)
}
case wtp: MethodType if !pt.isInstanceOf[SingletonType] =>
case wtp: MethodType if !pt.isSingleton =>
val arity =
if (functionExpected)
if (!isFullyDefined(pt, ForceDegree.none) && isFullyDefined(wtp, ForceDegree.none))
Expand Down
6 changes: 3 additions & 3 deletions compiler/test-resources/repl/patdef
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ scala> val (Const, List(`x`, _, a), b) = (0, List(0, 1337, 1), 2)
val a: Int = 1
val b: Int = 2
scala> val a@b = 0
val a: Int @unchecked = 0
val b: Int @unchecked = 0
val a: Int = 0
val b: Int = 0
scala> val Y = 2
val Y: Int = 2
scala> val X, Y = 3
Expand All @@ -15,7 +15,7 @@ val Y: Int = 3
scala> val foo = Y
val foo: Int = 3
scala> val X @ List(_) = List(1)
val X: List[Int] @unchecked = List(1)
val X: List[Int] = List(1)
scala> val _ @ _ = 1
scala> val _ @ List(x) = List(1)
val x: Int = 1
Expand Down
9 changes: 9 additions & 0 deletions tests/pos/i3564.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
case class Foo(s: "abc")

object Test {

val x: "abc" @deprecated = "abc"

val y: "abc" = x

}
9 changes: 9 additions & 0 deletions tests/pos/patdef.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object Test {

case class C(x: Int)

val c: Any = C(1)

val C(x) = c

}