Skip to content

Implement @alpha annotation #6597

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 6 commits into from
Jun 5, 2019
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
14 changes: 14 additions & 0 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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? */
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeErasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =>
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 31 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1124,6 +1138,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 {
Expand All @@ -1144,7 +1173,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 = ()
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
9 changes: 9 additions & 0 deletions tests/neg-custom-args/missing-alpha.scala
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 14 additions & 0 deletions tests/neg/alpha-early.scala
Original file line number Diff line number Diff line change
@@ -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

}
}
12 changes: 12 additions & 0 deletions tests/neg/alpha-late.scala
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 6 additions & 0 deletions tests/neg/alpha-override/B_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Java
public class B_1 {
public int bar() {
return 2;
}
}
4 changes: 4 additions & 0 deletions tests/neg/alpha-override/C_2.scala
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 6 additions & 0 deletions tests/neg/alpha-override1/B_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Java
public class B_1 {
public int bar() {
return 2;
}
}
7 changes: 7 additions & 0 deletions tests/neg/alpha-override1/D_2.scala
Original file line number Diff line number Diff line change
@@ -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
}
22 changes: 22 additions & 0 deletions tests/neg/alpha.scala
Original file line number Diff line number Diff line change
@@ -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

}
20 changes: 20 additions & 0 deletions tests/neg/doubleDefinition-late.scala
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 0 additions & 14 deletions tests/neg/doubleDefinition.check
Original file line number Diff line number Diff line change
Expand Up @@ -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.
13 changes: 1 addition & 12 deletions tests/neg/doubleDefinition.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
5 changes: 5 additions & 0 deletions tests/pos/alpha-override/A_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Scala
import annotation.alpha
class A_1 {
@alpha("bar") def foo(): Int = 1
}
7 changes: 7 additions & 0 deletions tests/pos/alpha-override/B_2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Java
public class B_2 extends A_1 {
@Override
public int bar() {
return 2;
}
}
8 changes: 8 additions & 0 deletions tests/pos/alpha.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import annotation.alpha

object Test {

def foo() = 1

@alpha("bar") def foo(x: Int) = 2
}
Loading