Skip to content

Add refining annotations #4626

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
Jun 8, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 25 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,31 @@ object Trees {

override def toText(printer: Printer) = printer.toText(this)

def sameTree(that: Tree[_]): Boolean = {
def isSame(x: Any, y: Any): Boolean =
x.asInstanceOf[AnyRef].eq(y.asInstanceOf[AnyRef]) || {
x match {
case x: Tree[_] =>
y match {
case y: Tree[_] => x.sameTree(y)
case _ => false
}
case x: List[_] =>
y match {
case y: List[_] => x.corresponds(y)(isSame)
case _ => false
}
case _ =>
false
}
}
this.getClass == that.getClass && {
val it1 = this.productIterator
val it2 = that.productIterator
it1.corresponds(it2)(isSame)
}
}

override def hashCode(): Int = uniqueId // for debugging; was: System.identityHashCode(this)
override def equals(that: Any) = this eq that.asInstanceOf[AnyRef]

Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Annotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ object Annotations {
def isEvaluated: Boolean = true

def ensureCompleted(implicit ctx: Context): Unit = tree

def sameAnnotation(that: Annotation)(implicit ctx: Context) =
symbol == that.symbol && tree.sameTree(that.tree)
}

case class ConcreteAnnotation(t: Tree) extends Annotation {
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,8 @@ class Definitions {
def ClassfileAnnotationClass(implicit ctx: Context) = ClassfileAnnotationType.symbol.asClass
lazy val StaticAnnotationType = ctx.requiredClassRef("scala.annotation.StaticAnnotation")
def StaticAnnotationClass(implicit ctx: Context) = StaticAnnotationType.symbol.asClass
lazy val RefiningAnnotationType = ctx.requiredClassRef("scala.annotation.RefiningAnnotation")
Copy link
Contributor

Choose a reason for hiding this comment

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

(Misaligned =)

def RefiningAnnotationClass(implicit ctx: Context) = RefiningAnnotationType.symbol.asClass

// Annotation classes
lazy val AliasAnnotType = ctx.requiredClassRef("scala.annotation.internal.Alias")
Expand Down
21 changes: 14 additions & 7 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
compareWild
case tp2: LazyRef =>
!tp2.evaluating && recur(tp1, tp2.ref)
case tp2: AnnotatedType =>
recur(tp1, tp2.tpe) // todo: refine?
case tp2: AnnotatedType if !tp2.isRefining =>
recur(tp1, tp2.tpe)
case tp2: ThisType =>
def compareThis = {
val cls2 = tp2.cls
Expand Down Expand Up @@ -345,7 +345,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
// because that would cause an assertionError. Return false instead.
// See i859.scala for an example where we hit this case.
!tp1.evaluating && recur(tp1.ref, tp2)
case tp1: AnnotatedType =>
case tp1: AnnotatedType if !tp1.isRefining =>
recur(tp1.tpe, tp2)
case AndType(tp11, tp12) =>
if (tp11.stripTypeVar eq tp12.stripTypeVar) recur(tp11, tp2)
Expand Down Expand Up @@ -567,6 +567,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
false
}
compareTypeBounds
case tp2: AnnotatedType if tp2.isRefining =>
(tp1.derivesAnnotWith(tp2.annot.sameAnnotation) || defn.isBottomType(tp1)) &&
recur(tp1, tp2.tpe)
case ClassInfo(pre2, cls2, _, _, _) =>
def compareClassInfo = tp1 match {
case ClassInfo(pre1, cls1, _, _, _) =>
Expand Down Expand Up @@ -661,6 +664,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
case _ =>
}
either(recur(tp11, tp2), recur(tp12, tp2))
case tp1: AnnotatedType if tp1.isRefining =>
isNewSubType(tp1.tpe)
case JavaArrayType(elem1) =>
def compareJavaArray = tp2 match {
case JavaArrayType(elem2) => isSubType(elem1, elem2)
Expand Down Expand Up @@ -700,7 +705,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
}
case tycon1: TypeVar =>
isMatchingApply(tycon1.underlying)
case tycon1: AnnotatedType =>
case tycon1: AnnotatedType if !tycon1.isRefining =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure when a type constructor would have a RefiningAnnotation, but wouldn't it in principle be okay to widen here, regardless of whether the annotation is refining?

Copy link
Contributor Author

@odersky odersky Jun 8, 2018

Choose a reason for hiding this comment

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

Yes, well spotted. Narrowing the lower bound is always safe, and it is the only thing we can do in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this PR was merged without addressing this comment?

isMatchingApply(tycon1.underlying)
case _ =>
false
Expand Down Expand Up @@ -811,7 +816,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
fourthTry
}
}
case _: TypeVar | _: AnnotatedType =>
case _: TypeVar =>
recur(tp1, tp2.superType)
case tycon2: AnnotatedType if !tycon2.isRefining =>
recur(tp1, tp2.superType)
case tycon2: AppliedType =>
fallback(tycon2.lowerBound)
Expand Down Expand Up @@ -1546,7 +1553,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
}
case tp1: TypeVar if tp1.isInstantiated =>
tp1.underlying & tp2
case tp1: AnnotatedType =>
case tp1: AnnotatedType if !tp1.isRefining =>
tp1.underlying & tp2
Copy link
Member

Choose a reason for hiding this comment

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

Why not rewrap the annotation around the intersection if it's a refining annotation?

case _ =>
NoType
Expand All @@ -1565,7 +1572,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
ExprType(rt1 | tp2.widenExpr)
case tp1: TypeVar if tp1.isInstantiated =>
tp1.underlying | tp2
case tp1: AnnotatedType =>
case tp1: AnnotatedType if !tp1.isRefining =>
tp1.underlying | tp2
case _ =>
NoType
Expand Down
20 changes: 20 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,15 @@ object Types {
case _ => false
}

/** Does this type have a supertype with an annotation satisfying given predicate `p`? */
def derivesAnnotWith(p: Annotation => Boolean)(implicit ctx: Context): Boolean = this match {
case tp: AnnotatedType => p(tp.annot) || tp.tpe.derivesAnnotWith(p)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming AnnotatedType#tpe to #parent or #info? (I find tp.tpe slightly confusing.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.

case tp: TypeProxy => tp.superType.derivesAnnotWith(p)
case AndType(l, r) => l.derivesAnnotWith(p) || r.derivesAnnotWith(p)
case OrType(l, r) => l.derivesAnnotWith(p) && r.derivesAnnotWith(p)
case _ => false
}

/** Does this type occur as a part of type `that`? */
final def occursIn(that: Type)(implicit ctx: Context): Boolean =
that existsPart (this == _)
Expand Down Expand Up @@ -3693,6 +3702,17 @@ object Types {

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

private[this] var isRefiningKnown = false
private[this] var isRefiningCache: Boolean = _

def isRefining(implicit ctx: Context) = {
if (!isRefiningKnown) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the cache be invalidated at the start of a new run?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about that too, but decided that it's probably fine given the internal nature of such annotations. Then again, we do expose RefiningAnnotation in the public scala.annotation package.

isRefiningCache = annot.symbol.derivesFrom(defn.RefiningAnnotationClass)
isRefiningKnown = true
}
isRefiningCache
}

override def iso(that: Any, bs: BinderPairs): Boolean = that match {
case that: AnnotatedType => tpe.equals(that.tpe, bs) && (annot `eq` that.annot)
case _ => false
Expand Down
3 changes: 3 additions & 0 deletions library/src/scala/annotation/RefiningAnnotation.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package scala.annotation

trait RefiningAnnotation extends StaticAnnotation
Copy link
Contributor

Choose a reason for hiding this comment

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

Why expose that in the stdlib, isn't this compiler internal only?

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, you might want to define your own annotations (say units of measure). Maybe they need a compiler plugin to order to be checked, or maybe you can do it in the future with annotation macros. RefiningAnnotation just makes sure they propagate correctly.