Skip to content

Fix #9050: Allow multidenotations with same signature #9063

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 20 commits into from
Jun 4, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ class CommunityBuildTest:
@Test def scodecBits = projects.scodecBits.run()
@Test def scodec = projects.scodec.run()
@Test def scalaParserCombinators = projects.scalaParserCombinators.run()
@Test def dottyCpsAsync = projects.dottyCpsAsync.run()
// blocked on #9074
//@Test def dottyCpsAsync = projects.dottyCpsAsync.run()
@Test def scalaz = projects.scalaz.run()
@Test def endpoints = projects.endpoints.run()
end CommunityBuildTest
Expand Down
427 changes: 130 additions & 297 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -922,8 +922,8 @@ object SymDenotations {
else true
}

if (pre eq NoPrefix) true
else if (isAbsent()) false
if pre eq NoPrefix then true
else if isAbsent() then false
else {
val boundary = accessBoundary(owner)

Expand Down Expand Up @@ -2100,7 +2100,7 @@ object SymDenotations {
var names = Set[Name]()
def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name
try {
for (p <- classParents)
for (p <- classParents if p.classSymbol.isClass)
for (name <- p.classSymbol.asClass.memberNames(keepOnly))
maybeAdd(name)
val ownSyms =
Expand Down
24 changes: 4 additions & 20 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2049,16 +2049,6 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
* instantiated TypeVars are dereferenced and annotations are stripped.
* Finally, refined types with the same refined name are
* opportunistically merged.
*
* Sometimes, the conjunction of two types cannot be formed because
* the types are in conflict of each other. In particular:
*
* 1. Two different class types are conflicting.
* 2. A class type conflicts with a type bounds that does not include the class reference.
* 3. Two method or poly types with different (type) parameters but the same
* signature are conflicting
*
* In these cases, a MergeError is thrown.
*/
final def andType(tp1: Type, tp2: Type, isErased: Boolean = ctx.erasedTypes): Type =
andTypeGen(tp1, tp2, AndType(_, _), isErased = isErased)
Expand All @@ -2072,10 +2062,6 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
* ExprType, LambdaType). Also, when forming an `|`,
* instantiated TypeVars are dereferenced and annotations are stripped.
*
* Sometimes, the disjunction of two types cannot be formed because
* the types are in conflict of each other. (@see `andType` for an enumeration
* of these cases). In cases of conflict a `MergeError` is raised.
*
* @param isErased Apply erasure semantics. If erased is true, instead of creating
* an OrType, the lub will be computed using TypeCreator#erasedLub.
*/
Expand Down Expand Up @@ -2141,13 +2127,11 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
// gives =:= types), but it keeps the type smaller.
tp2 match {
case tp2: RefinedType if tp1.refinedName == tp2.refinedName =>
try {
val jointInfo = Denotations.infoMeet(tp1.refinedInfo, tp2.refinedInfo, NoSymbol, NoSymbol, safeIntersection = false)
val jointInfo = Denotations.infoMeet(tp1.refinedInfo, tp2.refinedInfo, safeIntersection = false)
if jointInfo.exists then
tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, jointInfo)
}
catch {
case ex: MergeError => NoType
}
else
NoType
case _ =>
NoType
}
Expand Down
29 changes: 0 additions & 29 deletions compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -163,32 +163,3 @@ object CyclicReference {
}
}

class MergeError(val sym1: Symbol, val sym2: Symbol, val tp1: Type, val tp2: Type, prefix: Type) extends TypeError {

private def showSymbol(sym: Symbol)(implicit ctx: Context): String =
if (sym.exists) sym.showLocated else "[unknown]"

private def showType(tp: Type)(implicit ctx: Context) = tp match {
case ClassInfo(_, cls, _, _, _) => cls.showLocated
case _ => tp.show
}

protected def addendum(implicit ctx: Context): String =
if (prefix `eq` NoPrefix) ""
else {
val owner = prefix match {
case prefix: ThisType => prefix.cls.show
case prefix: TermRef => prefix.symbol.show
case _ => i"type $prefix"
}
s"\nas members of $owner"
}

override def produceMessage(implicit ctx: Context): Message = {
if (ctx.debug) printStackTrace()
i"""cannot merge
| ${showSymbol(sym1)} of type ${showType(tp1)} and
| ${showSymbol(sym2)} of type ${showType(tp2)}$addendum
"""
}
}
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ object Types {
pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, jointInfo)
}
else
val joint = pdenot & (
val joint = pdenot.meet(
new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre),
pre,
safeIntersection = ctx.base.pendingMemberSearches.contains(name))
Expand Down Expand Up @@ -726,7 +726,7 @@ object Types {
}

def goAnd(l: Type, r: Type) =
go(l) & (go(r), pre, safeIntersection = ctx.base.pendingMemberSearches.contains(name))
go(l).meet(go(r), pre, safeIntersection = ctx.base.pendingMemberSearches.contains(name))

def goOr(tp: OrType) = tp match {
case OrUncheckedNull(tp1) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class TastyPrinter(bytes: Array[Byte])(implicit ctx: Context) {
printName(); printName()
case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM | NAMEDARG | BIND =>
printName(); printTrees()
case REFINEDtype | TERMREFin | TYPEREFin =>
case REFINEDtype | TERMREFin | TYPEREFin | SELECTin =>
printName(); printTree(); printTrees()
case RETURN | HOLE =>
printNat(); printTrees()
Expand Down
30 changes: 21 additions & 9 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import dotty.tools.tasty.TastyBuffer._
import ast.Trees._
import ast.{untpd, tpd}
import Contexts._, Symbols._, Types._, Names._, Constants._, Decorators._, Annotations._, Flags._
import Denotations.MultiDenotation
import typer.Inliner
import NameKinds._
import StdNames.nme
Expand Down Expand Up @@ -173,22 +174,20 @@ class TreePickler(pickler: TastyPickler) {
case tpe: NamedType =>
val sym = tpe.symbol
def pickleExternalRef(sym: Symbol) = {
def pickleCore() = {
pickleNameAndSig(sym.name, tpe.signature)
pickleType(tpe.prefix)
}
val isShadowedRef =
sym.isClass && tpe.prefix.member(sym.name).symbol != sym
if (sym.is(Flags.Private) || isShadowedRef) {
writeByte(if (tpe.isType) TYPEREFin else TERMREFin)
withLength {
pickleCore()
pickleNameAndSig(sym.name, tpe.symbol.signature)
pickleType(tpe.prefix)
pickleType(sym.owner.typeRef)
}
}
else {
writeByte(if (tpe.isType) TYPEREF else TERMREF)
pickleCore()
pickleNameAndSig(sym.name, tpe.signature)
pickleType(tpe.prefix)
}
}
if (sym.is(Flags.Package)) {
Expand Down Expand Up @@ -381,10 +380,23 @@ class TreePickler(pickler: TastyPickler) {
pickleType(tp)
}
case _ =>
writeByte(if (name.isTypeName) SELECTtpt else SELECT)
val sig = tree.tpe.signature
pickleNameAndSig(name, sig)
pickleTree(qual)
val isAmbiguous =
sig != Signature.NotAMethod
&& qual.tpe.nonPrivateMember(name).match
case d: MultiDenotation => d.atSignature(sig).isInstanceOf[MultiDenotation]
case _ => false
if isAmbiguous then
writeByte(SELECTin)
withLength {
pickleNameAndSig(name, tree.symbol.signature)
pickleTree(qual)
pickleType(tree.symbol.owner.typeRef)
}
else
writeByte(if (name.isTypeName) SELECTtpt else SELECT)
pickleNameAndSig(name, sig)
pickleTree(qual)
}
case Apply(fun, args) =>
if (fun.symbol eq defn.throwMethod) {
Expand Down
29 changes: 22 additions & 7 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Symbols._
import Types._
import Scopes._
import SymDenotations._
import Denotations._
import Names._
import NameOps._
import StdNames._
Expand Down Expand Up @@ -330,12 +331,12 @@ class TreeUnpickler(reader: TastyReader,
case TERMREFin =>
var sname = readName()
val prefix = readType()
val space = readType()
val owner = readType()
sname match {
case SignedName(name, sig) =>
TermRef(prefix, name, space.decl(name).asSeenFrom(prefix).atSignature(sig))
TermRef(prefix, name, owner.decl(name).atSignature(sig).asSeenFrom(prefix))
case name =>
TermRef(prefix, name, space.decl(name).asSeenFrom(prefix))
TermRef(prefix, name, owner.decl(name).asSeenFrom(prefix))
}
case TYPEREFin =>
val name = readName().toTypeName
Expand Down Expand Up @@ -1040,10 +1041,8 @@ class TreeUnpickler(reader: TastyReader,
}
}

def completeSelect(name: Name, sig: Signature): Select = {
val qual = readTerm()(ctx)
def makeSelect(qual: Tree, name: Name, denot: Denotation): Select =
var qualType = qual.tpe.widenIfUnstable
val denot = accessibleDenot(qualType, name, sig)
val owner = denot.symbol.maybeOwner
if (owner.isPackageObject && qualType.termSymbol.is(Package))
qualType = qualType.select(owner.sourceModule)
Expand All @@ -1052,7 +1051,11 @@ class TreeUnpickler(reader: TastyReader,
case name: TermName => TermRef(qualType, name, denot)
}
ConstFold(untpd.Select(qual, name).withType(tpe))
}

def completeSelect(name: Name, sig: Signature): Select =
val qual = readTerm()(ctx)
val denot = accessibleDenot(qual.tpe.widenIfUnstable, name, sig)
makeSelect(qual, name, denot)

def readQualId(): (untpd.Ident, TypeRef) =
val qual = readTerm().asInstanceOf[untpd.Ident]
Expand Down Expand Up @@ -1165,6 +1168,18 @@ class TreeUnpickler(reader: TastyReader,
case SELECTouter =>
val levels = readNat()
readTerm().outerSelect(levels, SkolemType(readType()))
case SELECTin =>
var sname = readName()
val qual = readTerm()
val owner = readType()
def select(name: Name, denot: Denotation) =
val prefix = ctx.typeAssigner.maybeSkolemizePrefix(qual.tpe.widenIfUnstable, name)
makeSelect(qual, name, denot.asSeenFrom(prefix))
sname match
case SignedName(name, sig) =>
select(name, owner.decl(name).atSignature(sig))
case name =>
select(name, owner.decl(name))
case REPEATED =>
val elemtpt = readTpt()
SeqLiteral(until(end)(readTerm()), elemtpt)
Expand Down
20 changes: 4 additions & 16 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -425,22 +425,10 @@ object RefChecks {
}*/
}

try {
val opc = new OverridingPairs.Cursor(clazz)
while (opc.hasNext) {
checkOverride(opc.overriding, opc.overridden)
opc.next()
}
}
catch {
case ex: MergeError =>
val addendum = ex.tp1 match {
case tp1: ClassInfo =>
"\n(Note that having same-named member classes in types of a mixin composition is no longer allowed)"
case _ => ""
}
ctx.error(ex.getMessage + addendum, clazz.sourcePos)
}
val opc = new OverridingPairs.Cursor(clazz)
while opc.hasNext do
checkOverride(opc.overriding, opc.overridden)
opc.next()
printMixinOverrideErrors()

// Verifying a concrete class has nothing unimplemented.
Expand Down
13 changes: 10 additions & 3 deletions tasty/src/dotty/tools/tasty/TastyFormat.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ Standard-Section: "ASTs" TopLevelStat*
Term = Path -- Paths represent both types and terms
IDENT NameRef Type -- Used when term ident’s type is not a TermRef
SELECT possiblySigned_NameRef qual_Term -- qual.name
SELECTin Length possiblySigned_NameRef qual_Term owner_Type -- qual.name, referring to a symbol declared in owner that has the given signature (see note below)
QUALTHIS typeIdent_Tree -- id.this, different from THIS in that it contains a qualifier ident with position.
NEW clsType_Term -- new cls
THROW throwableExpr_Term -- throw throwableExpr
Expand Down Expand Up @@ -122,7 +123,7 @@ Standard-Section: "ASTs" TopLevelStat*
TERMREFsymbol sym_ASTRef qual_Type -- A reference `qual.sym` to a local member with prefix `qual`
TERMREFpkg fullyQualified_NameRef -- A reference to a package member with given fully qualified name
TERMREF possiblySigned_NameRef qual_Type -- A reference `qual.name` to a non-local member
TERMREFin Length possiblySigned_NameRef qual_Type namespace_Type -- A reference `qual.name` to a non-local member that's private in `namespace`
TERMREFin Length possiblySigned_NameRef qual_Type owner_Type -- A reference `qual.name` referring to a non-local symbol declared in owner that has the given signature (see note below)
Copy link
Member

Choose a reason for hiding this comment

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

Should the documentation for TYPEREFin be updated in a similar way?

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 since TypeRefs never take a signature

THIS clsRef_Type -- cls.this
RECthis recType_ASTRef -- The `this` in a recursive refined type `recType`.
SHAREDtype path_ASTRef -- link to previously serialized path
Expand Down Expand Up @@ -212,6 +213,10 @@ Standard-Section: "ASTs" TopLevelStat*

Annotation = ANNOTATION Length tycon_Type fullAnnotation_Term -- An annotation, given (class) type of constructor, and full application tree

Note: The signature of a SELECTin or TERMREFin node is the signature of the selected symbol,
not the signature of the reference. The latter undergoes an asSeenFrom but the former
does not.

Note: Tree tags are grouped into 5 categories that determine what follows, and thus allow to compute the size of the tagged tree in a generic way.

Category 1 (tags 1-49) : tag
Expand Down Expand Up @@ -248,7 +253,7 @@ Standard Section: "Comments" Comment*
object TastyFormat {

final val header: Array[Int] = Array(0x5C, 0xA1, 0xAB, 0x1F)
val MajorVersion: Int = 22
val MajorVersion: Int = 23
val MinorVersion: Int = 0

/** Tags used to serialize names, should update [[nameTagToString]] if a new constant is added */
Expand Down Expand Up @@ -452,6 +457,7 @@ object TastyFormat {
final val ANNOTATION = 173
final val TERMREFin = 174
final val TYPEREFin = 175
final val SELECTin = 176

final val METHODtype = 180

Expand Down Expand Up @@ -646,6 +652,7 @@ object TastyFormat {
case SUPERtype => "SUPERtype"
case TERMREFin => "TERMREFin"
case TYPEREFin => "TYPEREFin"
case SELECTin => "SELECTin"

case REFINEDtype => "REFINEDtype"
case REFINEDtpt => "REFINEDtpt"
Expand Down Expand Up @@ -675,7 +682,7 @@ object TastyFormat {
*/
def numRefs(tag: Int): Int = tag match {
case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM | NAMEDARG | RETURN | BIND |
SELFDEF | REFINEDtype | TERMREFin | TYPEREFin | HOLE => 1
SELFDEF | REFINEDtype | TERMREFin | TYPEREFin | SELECTin | HOLE => 1
case RENAMED | PARAMtype => 2
case POLYtype | TYPELAMBDAtype | METHODtype => -1
case _ => 0
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-custom-args/allow-double-bindings/i1240.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class C[T] {

object C {
def main(args: Array[String]) =
new C[D]().foo(new D()) // error: ambiguous
new C[D]().foo(new D())
}

class C1[T] {
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i1240b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ abstract class A[X] extends T[X] {
trait U[X] extends T[X] {
abstract override def foo(x: X): X = super.foo(x)
}
object Test extends A[String] with U[String] // error: accidental override // error: merge error
object Test extends A[String] with U[String] // error: accidental override
2 changes: 1 addition & 1 deletion tests/neg/i4470a.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
object RepeatedEnum {

enum Maybe { // error
case Foo // error
case Foo
}

enum Maybe { // error
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i4470b.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
object RepeatedExtendEnum {

enum Maybe[T] derives Eql { // error // error
enum Maybe[T] derives Eql { // error
case Foo extends Maybe[Int]
}

Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i4470c.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
object DuplicatedEnum {
enum Maybe[+T] { // error
case Some(x: T) // error
case Some(x: T)
}

enum Maybe[+T] { // error
Expand Down
Loading