Merge pull request #6597 from dotty-staging/add-alpha

odersky · web-flow · commit d9c04cc141ef · 2019-06-05T15:28:07.000+02:00
Implement @Alpha annotation
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -5,11 +5,13 @@ package core
 import Periods._, Contexts._, Symbols._, Denotations._, Names._, NameOps._, Annotations._
 import Types._, Flags._, Decorators._, DenotTransformers._, StdNames._, Scopes._
 import NameOps._, NameKinds._, Phases._
+import Constants.Constant
 import TypeApplications.TypeParamInfo
 import Scopes.Scope
 import dotty.tools.io.AbstractFile
 import Decorators.SymbolIteratorDecorator
 import ast._
+import Trees.Literal
 import annotation.tailrec
 import util.SimpleIdentityMap
 import util.Stats
@@ -442,6 +444,18 @@ object SymDenotations {
     /** `fullName` where `.' is the separator character */
     def fullName(implicit ctx: Context): Name = fullNameSeparated(QualifiedName)
 
+    /** The name given in an `@alpha` annotation if one is present, `name` otherwise */
+    final def erasedName(implicit ctx: Context): Name =
+      getAnnotation(defn.AlphaAnnot) match {
+        case Some(ann) =>
+          ann.arguments match {
+            case Literal(Constant(str: String)) :: Nil =>
+              if (isType) str.toTypeName else str.toTermName
+            case _ => name
+          }
+        case _ => name
+      }
+
     // ----- Tests -------------------------------------------------
 
     /** Is this denotation a type? */
diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala
@@ -537,7 +537,11 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
   private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type = {
     val cls = tref.symbol.asClass
     val underlying = underlyingOfValueClass(cls)
-    if (underlying.exists && !isCyclic(cls)) ErasedValueType(tref, valueErasure(underlying))
+    if (underlying.exists && !isCyclic(cls)) {
+      val erasedValue = valueErasure(underlying)
+      assert(erasedValue.exists, i"no erasure for $underlying")
+      ErasedValueType(tref, erasedValue)
+    }
     else NoType
   }
 
@@ -605,7 +609,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
       case tp: TypeVar =>
         val inst = tp.instanceOpt
         if (inst.exists) sigName(inst) else tpnme.Uninstantiated
-      case tp @ RefinedType(parent, nme.apply, _) if parent.typeSymbol eq defn.PolyFunctionClass => 
+      case tp @ RefinedType(parent, nme.apply, _) if parent.typeSymbol eq defn.PolyFunctionClass =>
         // we need this case rather than falling through to the default
         // because RefinedTypes <: TypeProxy and it would be caught by
         // the case immediately below
diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
@@ -2145,6 +2145,7 @@ object messages {
   case class DoubleDefinition(decl: Symbol, previousDecl: Symbol, base: Symbol)(implicit ctx: Context) extends Message(DoubleDefinitionID) {
     val kind: String = "Duplicate Symbol"
     val msg: String = {
+      def nameAnd = if (decl.name != previousDecl.name) " name and" else ""
       val details = if (decl.isRealMethod && previousDecl.isRealMethod) {
         // compare the signatures when both symbols represent methods
         decl.signature.matchDegree(previousDecl.signature) match {
@@ -2153,7 +2154,7 @@ object messages {
           case Signature.ParamMatch =>
             "have matching parameter types."
           case Signature.FullMatch =>
-            "have the same type after erasure."
+            i"have the same$nameAnd type after erasure."
         }
       } else ""
       def symLocation(sym: Symbol) = {
diff --git a/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala b/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala
@@ -8,6 +8,7 @@ import Types._, Contexts._, Flags._, DenotTransformers._
 import Symbols._, StdNames._, Trees._
 import TypeErasure.ErasedValueType, ValueClasses._
 import reporting.diagnostic.messages.DoubleDefinition
+import NameKinds.SuperAccessorName
 
 object ElimErasedValueType {
   val name: String = "elimErasedValueType"
@@ -95,7 +96,11 @@ class ElimErasedValueType extends MiniPhase with InfoTransformer { thisPhase =>
         (sym1.owner.derivesFrom(defn.PolyFunctionClass) ||
          sym2.owner.derivesFrom(defn.PolyFunctionClass))
 
-      if (!info1.matchesLoosely(info2) && !bothPolyApply)
+      // super-accessors start as private, and their expanded name can clash after
+      // erasure. TODO: Verify that this is OK.
+      def bothSuperAccessors = sym1.name.is(SuperAccessorName) && sym2.name.is(SuperAccessorName)
+      if (sym1.name != sym2.name && !bothSuperAccessors ||
+          !info1.matchesLoosely(info2) && !bothPolyApply)
         ctx.error(DoubleDefinition(sym1, sym2, root), root.sourcePos)
     }
     val earlyCtx = ctx.withPhase(ctx.elimRepeatedPhase.next)
diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -69,6 +69,8 @@ class Erasure extends Phase with DenotTransformer {
         else oldSymbol
         val oldOwner = ref.owner
         val newOwner = if (oldOwner eq defn.AnyClass) defn.ObjectClass else oldOwner
+        val oldName = ref.name
+        val newName = ref.erasedName
         val oldInfo = ref.info
         val newInfo = transformInfo(oldSymbol, oldInfo)
         val oldFlags = ref.flags
@@ -77,10 +79,11 @@ class Erasure extends Phase with DenotTransformer {
           else oldFlags &~ Flags.HasDefaultParams // HasDefaultParams needs to be dropped because overriding might become overloading
 
         // TODO: define derivedSymDenotation?
-        if ((oldSymbol eq newSymbol) && (oldOwner eq newOwner) && (oldInfo eq newInfo) && (oldFlags == newFlags)) ref
+        if ((oldSymbol eq newSymbol) && (oldOwner eq newOwner) && (oldName eq newName) && (oldInfo eq newInfo) && (oldFlags == newFlags))
+          ref
         else {
           assert(!ref.is(Flags.PackageClass), s"trans $ref @ ${ctx.phase} oldOwner = $oldOwner, newOwner = $newOwner, oldInfo = $oldInfo, newInfo = $newInfo ${oldOwner eq newOwner} ${oldInfo eq newInfo}")
-          ref.copySymDenotation(symbol = newSymbol, owner = newOwner, initFlags = newFlags, info = newInfo)
+          ref.copySymDenotation(symbol = newSymbol, owner = newOwner, name = newName, initFlags = newFlags, info = newInfo)
         }
       }
     case ref: JointRefDenotation =>
diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala
@@ -111,6 +111,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
 
     private def processMemberDef(tree: Tree)(implicit ctx: Context): tree.type = {
       val sym = tree.symbol
+      Checking.checkValidOperator(sym)
       sym.transformAnnotations(transformAnnot)
       sym.defTree = tree
       tree
diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala
@@ -274,6 +274,20 @@ object Checking {
     }
   }
 
+  /** If `sym` has an operator name, check that it has an @alpha annotation under -strict */
+  def checkValidOperator(sym: Symbol)(implicit ctx: Context): Unit =
+    sym.name.toTermName match {
+      case name: SimpleName
+      if name.exists(isOperatorPart) &&
+        !sym.getAnnotation(defn.AlphaAnnot).isDefined &&
+        !sym.is(Synthetic) &&
+        !name.isConstructorName &&
+        ctx.settings.strict.value =>
+        ctx.deprecationWarning(
+          i"$sym has an operator name; it should come with an @alpha annotation", sym.sourcePos)
+      case _ =>
+    }
+
   /** Check that `info` of symbol `sym` is not cyclic.
    *  @pre     sym is not yet initialized (i.e. its type is a Completer).
    *  @return  `info` where every legal F-bounded reference is proctected
@@ -864,7 +878,7 @@ trait Checking {
         def javaFieldMethodPair =
           decl.is(JavaDefined) && other.is(JavaDefined) &&
           decl.is(Method) != other.is(Method)
-        if ((decl.signature.clashes(other.signature) || decl.matches(other)) && !javaFieldMethodPair) {
+        if (decl.matches(other) && !javaFieldMethodPair) {
           def doubleDefError(decl: Symbol, other: Symbol): Unit =
             if (!decl.info.isErroneous && !other.info.isErroneous)
               ctx.error(DoubleDefinition(decl, other, cls), decl.sourcePos)
@@ -1130,6 +1144,21 @@ trait Checking {
       case _ =>
     }
   }
+
+  /** Check that symbol's external name does not clash with symbols defined in the same scope */
+  def checkNoAlphaConflict(stats: List[Tree])(implicit ctx: Context): Unit = {
+    var seen = Set[Name]()
+    for (stat <- stats) {
+      val sym = stat.symbol
+      val ename = sym.erasedName
+      if (ename != sym.name) {
+        val preExisting = ctx.effectiveScope.lookup(ename)
+        if (preExisting.exists || seen.contains(ename))
+          ctx.error(em"@alpha annotation ${'"'}$ename${'"'} clashes with other definition is same scope", stat.sourcePos)
+        seen += ename
+      }
+    }
+  }
 }
 
 trait ReChecking extends Checking {
@@ -1150,7 +1179,7 @@ trait NoChecking extends ReChecking {
   override def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit = ()
   override def checkFeasibleParent(tp: Type, pos: SourcePosition, where: => String = "")(implicit ctx: Context): Type = tp
   override def checkInlineConformant(tree: Tree, isFinal: Boolean, what: => String)(implicit ctx: Context): Unit = ()
-  override def checkNoDoubleDeclaration(cls: Symbol)(implicit ctx: Context): Unit = ()
+  override def checkNoAlphaConflict(stats: List[Tree])(implicit ctx: Context): Unit = ()
   override def checkParentCall(call: Tree, caller: ClassSymbol)(implicit ctx: Context): Unit = ()
   override def checkSimpleKinded(tpt: Tree)(implicit ctx: Context): Tree = tpt
   override def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context): Unit = ()
diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -408,6 +408,11 @@ object RefChecks {
       } else if (!compatibleTypes(memberTp(self), otherTp(self)) &&
                  !compatibleTypes(memberTp(upwardsSelf), otherTp(upwardsSelf))) {
         overrideError("has incompatible type" + err.whyNoMatchStr(memberTp(self), otherTp(self)))
+      } else if (member.erasedName != other.erasedName) {
+        if (other.erasedName != other.name)
+          overrideError(i"needs to be declared with @alpha(${"\""}${other.erasedName}${"\""}) so that external names match")
+        else
+          overrideError("cannot have an @alpha annotation since external names would be different")
       } else {
         checkOverrideDeprecated()
       }
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -2312,7 +2312,9 @@ class Typer extends Namer
       case _ =>
         stat
     }
-    traverse(stats)(localCtx).mapConserve(finalize)
+    val stats1 = traverse(stats)(localCtx).mapConserve(finalize)
+    if (ctx.owner == exprOwner) checkNoAlphaConflict(stats1)
+    stats1
   }
 
   /** Given an inline method `mdef`, the method rewritten so that its body
diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala
@@ -180,7 +180,8 @@ class CompilationTests extends ParallelTesting {
         "tests/neg-custom-args/toplevel-samesource/nested/S.scala"),
         defaultOptions),
       compileFile("tests/neg-custom-args/i6300.scala", allowDeepSubtypes),
-      compileFile("tests/neg-custom-args/infix.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings"))
+      compileFile("tests/neg-custom-args/infix.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")),
+      compileFile("tests/neg-custom-args/missing-alpha.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings"))
     ).checkExpectedErrors()
   }
 
diff --git a/tests/neg-custom-args/missing-alpha.scala b/tests/neg-custom-args/missing-alpha.scala
@@ -0,0 +1,9 @@
+// Compile with -strict -Xfatal-warnings -deprecation
+import scala.annotation.alpha
+class & {   // error
+
+  @alpha("op") def *(x: Int): Int = ???  // OK
+  def / (x: Int): Int       // error
+  val frozen_& : Int = ???  // error
+  object some_???           // error
+}
diff --git a/tests/neg/alpha-early.scala b/tests/neg/alpha-early.scala
@@ -0,0 +1,14 @@
+
+import annotation.alpha
+
+class Gamma {
+
+  def foo2() = {
+    def bar = 1
+    @alpha("bar") def baz = 2 // error: @alpha annotation clashes
+
+    @alpha("bam") def x1 = 0
+    @alpha("bam") def y1 = 0 // error: @alpha annotation clashes
+
+  }
+}
diff --git a/tests/neg/alpha-late.scala b/tests/neg/alpha-late.scala
@@ -0,0 +1,12 @@
+
+import annotation.alpha
+
+class Gamma {
+
+  def foo: Int = 1
+
+}
+
+class Delta extends Gamma {      // error: name clash
+  @alpha("foo") def bar: Int = 1
+}
diff --git a/tests/neg/alpha-override/B_1.java b/tests/neg/alpha-override/B_1.java
@@ -0,0 +1,6 @@
+// Java
+public class B_1 {
+  public int bar() {
+    return 2;
+  }
+}
diff --git a/tests/neg/alpha-override/C_2.scala b/tests/neg/alpha-override/C_2.scala
@@ -0,0 +1,4 @@
+import annotation.alpha
+class C extends B_1 {   // error: Name clash between defined and inherited member
+  @alpha("bar") def foo(): Int = 3
+}
diff --git a/tests/neg/alpha-override1/B_1.java b/tests/neg/alpha-override1/B_1.java
@@ -0,0 +1,6 @@
+// Java
+public class B_1 {
+  public int bar() {
+    return 2;
+  }
+}
diff --git a/tests/neg/alpha-override1/D_2.scala b/tests/neg/alpha-override1/D_2.scala
@@ -0,0 +1,7 @@
+import annotation.alpha
+class D extends B_1 {
+  @alpha("bar") def foo(): Int = 3
+}
+class E extends B_1 {
+  @alpha("baz") override def bar(): Int = 3 // error: cannot have an @alpha annotation since external names would be different
+}
diff --git a/tests/neg/alpha.scala b/tests/neg/alpha.scala
@@ -0,0 +1,22 @@
+import annotation.alpha
+
+
+abstract class Alpha[T] {
+
+  def foo() = 1
+
+  @alpha("bar") def foo(x: T): T
+
+  @alpha("append") def ++ (xs: Alpha[T]): Alpha[T] = this
+
+}
+
+class Beta extends Alpha[String] {
+
+  @alpha("foo1") override def foo() = 1    // error
+
+  @alpha("baz") def foo(x: String): String = x ++ x   // error
+
+  override def ++ (xs: Alpha[String]): Alpha[String] = this   // error
+
+}
diff --git a/tests/neg/doubleDefinition-late.scala b/tests/neg/doubleDefinition-late.scala
@@ -0,0 +1,20 @@
+
+import annotation.alpha
+
+class Gamma {
+
+  val v = 1
+  @alpha("v") val w = 2   // error: double definition
+
+}
+// Error when overloading polymorphic and non-polymorphic methods
+class Test19 {
+  def foo[T <: Int](x: T): T = x
+  def foo(x: Int): Int = x // error
+}
+
+// Error when overloading polymorphic methods
+class Test20 {
+  def foo[T <: Int](x: T): T = x
+  def foo[S <: Int, T <: Int](x: S): T = ??? // error
+}
diff --git a/tests/neg/doubleDefinition.check b/tests/neg/doubleDefinition.check
@@ -122,17 +122,3 @@
     |	    Double definition:
     |	    var foo: Int in class Test16 at line 115 and
     |	    def foo: => Int in class Test16 at line 116
--- [E120] Duplicate Symbol Error: tests/neg/doubleDefinition.scala:138:6 -----------------------------------------------
-138 |  def foo(x: Int): Int = x // error
-    |      ^
-    |      Double definition:
-    |      def foo: [T <: Int](x: T): T in class Test19 at line 137 and
-    |      def foo(x: Int): Int in class Test19 at line 138
-    |      have the same type after erasure.
--- [E120] Duplicate Symbol Error: tests/neg/doubleDefinition.scala:144:6 -----------------------------------------------
-144 |  def foo[S <: Int, T <: Int](x: S): T = ??? // error
-    |      ^
-    |      Double definition:
-    |      def foo: [T <: Int](x: T): T in class Test20 at line 143 and
-    |      def foo: [S <: Int, T <: Int](x: S): T in class Test20 at line 144
-    |      have the same type after erasure.
diff --git a/tests/neg/doubleDefinition.scala b/tests/neg/doubleDefinition.scala
@@ -82,7 +82,7 @@ class Test8d {
 	def foo = 2 // error
 }
 
-// test method and contructor argument clashing
+// test method and constructor argument clashing
 
 class Test9(val foo: Int) {
 	def foo: String // error
@@ -132,14 +132,3 @@ class Test18 {
 	def foo(b: B) = 1
 }
 
-// Error when overloading polymorphic and non-polymorphic methods
-class Test19 {
-  def foo[T <: Int](x: T): T = x
-  def foo(x: Int): Int = x // error
-}
-
-// Error when overloading polymorphic methods
-class Test20 {
-  def foo[T <: Int](x: T): T = x
-  def foo[S <: Int, T <: Int](x: S): T = ??? // error
-}
diff --git a/tests/pos/alpha-override/A_1.scala b/tests/pos/alpha-override/A_1.scala
@@ -0,0 +1,5 @@
+// Scala
+import annotation.alpha
+class A_1 {
+  @alpha("bar") def foo(): Int = 1
+}
diff --git a/tests/pos/alpha-override/B_2.java b/tests/pos/alpha-override/B_2.java
@@ -0,0 +1,7 @@
+// Java
+public class B_2 extends A_1 {
+  @Override
+  public int bar() {
+    return 2;
+  }
+}
diff --git a/tests/pos/alpha.scala b/tests/pos/alpha.scala
@@ -0,0 +1,8 @@
+import annotation.alpha
+
+object Test {
+
+  def foo() = 1
+
+  @alpha("bar") def foo(x: Int) = 2
+}
diff --git a/tests/run/alpha-interop/Test_2.java b/tests/run/alpha-interop/Test_2.java
diff --git a/tests/run/alpha-interop/Test_3.scala b/tests/run/alpha-interop/Test_3.scala
diff --git a/tests/run/alpha-interop/alpha_1.scala b/tests/run/alpha-interop/alpha_1.scala
diff --git a/tests/run/alpha.scala b/tests/run/alpha.scala

Original file line number	Diff line number	Diff line change
`@@ -2312,7 +2312,9 @@ class Typer extends Namer`
`2312`	`2312`	`case _ =>`
`2313`	`2313`	`stat`
`2314`	`2314`	`}`
`2315`		`- traverse(stats)(localCtx).mapConserve(finalize)`
	`2315`	`+ val stats1 = traverse(stats)(localCtx).mapConserve(finalize)`
	`2316`	`+ if (ctx.owner == exprOwner) checkNoAlphaConflict(stats1)`
	`2317`	`+ stats1`
`2316`	`2318`	`}`
`2317`	`2319`
`2318`	`2320`	/** Given an inline method `mdef`, the method rewritten so that its body
Original file line number	Diff line number	Diff line change
`@@ -180,7 +180,8 @@ class CompilationTests extends ParallelTesting {`
`180`	`180`	`"tests/neg-custom-args/toplevel-samesource/nested/S.scala"),`
`181`	`181`	`defaultOptions),`
`182`	`182`	`compileFile("tests/neg-custom-args/i6300.scala", allowDeepSubtypes),`
`183`		`- compileFile("tests/neg-custom-args/infix.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings"))`
	`183`	`+ compileFile("tests/neg-custom-args/infix.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")),`
	`184`	`+ compileFile("tests/neg-custom-args/missing-alpha.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings"))`
`184`	`185`	`).checkExpectedErrors()`
`185`	`186`	`}`
`186`	`187`