From 668f3eb5ce847eda235cc847d660ee70d6fea48e Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jan 2023 14:10:35 +0100 Subject: [PATCH 1/3] Drop requirement that self types are closed #702 introduced a requirement that self types are closed. This means > If trait X has self type S and C is a class symbol of S, then S also conforms to the self type of C. An example that violates this requirement is ```scala trait X { self: Y => } // error: missing requirement: self type Y & X of trait X does not conform to self type Z of required trait Y trait Y { self: Z => } trait Z ``` But it's no longer clear what the requirement should achieve. If we let the example above typecheck and try to implement X with something like ```scala class C extends X, Y ``` we would at that point get an error saying that `C` does not conform to the self type Z of Y. So it would have to be ```scala class C extends X, Y, Z ``` and this one looks fine. The original change in #702 was made to avoid a crash in pending/run/t7933.scala. Unfortunately, we cannot reproduce this anymore since it depends on nsc.interpreter, which is no longer part of Scala 2.13. Since we are no longer sure what the restriction should achieve I think it's better to drop it for now. If people discover problems with code that uses "open" self types, we can try to fix those problems, and if that does not work, would fallback re-instituting the restriction. It's not ideal, but I don't see another way. --- .../dotty/tools/dotc/core/SymDenotations.scala | 17 ++--------------- .../src/dotty/tools/dotc/typer/RefChecks.scala | 9 +++------ tests/neg/selfInheritance.scala | 4 ---- tests/pos/i16407.scala | 7 +++++++ tests/pos/open-selftype.scala | 8 ++++++++ 5 files changed, 20 insertions(+), 25 deletions(-) create mode 100644 tests/pos/i16407.scala create mode 100644 tests/pos/open-selftype.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index e267bc51758f..2b5c51188571 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -702,21 +702,8 @@ object SymDenotations { flagsUNSAFE.is(Provisional) // do not force the info to check the flag /** Is this the denotation of a self symbol of some class? - * This is the case if one of two conditions holds: - * 1. It is the symbol referred to in the selfInfo part of the ClassInfo - * which is the type of this symbol's owner. - * 2. This symbol is owned by a class, it's selfInfo field refers to a type - * (indicating the self definition does not introduce a name), and the - * symbol's name is "_". - * TODO: Find a more robust way to characterize self symbols, maybe by - * spending a Flag on them? - */ - final def isSelfSym(using Context): Boolean = owner.infoOrCompleter match { - case ClassInfo(_, _, _, _, selfInfo) => - selfInfo == symbol || - selfInfo.isInstanceOf[Type] && name == nme.WILDCARD - case _ => false - } + */ + final def isSelfSym(using Context): Boolean = is(SelfName) /** Is this definition contained in `boundary`? * Same as `ownersIterator contains boundary` but more efficient. diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index f7c7bfa2f8a1..ad32af71fc0d 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -102,18 +102,15 @@ object RefChecks { withMode(Mode.CheckBoundsOrSelfType) { val cinfo = cls.classInfo - def checkSelfConforms(other: ClassSymbol, category: String, relation: String) = + def checkSelfConforms(other: ClassSymbol) = val otherSelf = other.declaredSelfTypeAsSeenFrom(cls.thisType) if otherSelf.exists then if !(cinfo.selfType <:< otherSelf) then - report.error(DoesNotConformToSelfType(category, cinfo.selfType, cls, otherSelf, relation, other), + report.error(DoesNotConformToSelfType("illegal inheritance", cinfo.selfType, cls, otherSelf, "parent", other), cls.srcPos) for psym <- parents do - checkSelfConforms(psym.asClass, "illegal inheritance", "parent") - for reqd <- cls.asClass.givenSelfType.classSymbols do - if reqd != cls then - checkSelfConforms(reqd, "missing requirement", "required") + checkSelfConforms(psym.asClass) } end checkSelfAgainstParents diff --git a/tests/neg/selfInheritance.scala b/tests/neg/selfInheritance.scala index 073316de008c..e8eb2bab5624 100644 --- a/tests/neg/selfInheritance.scala +++ b/tests/neg/selfInheritance.scala @@ -26,7 +26,3 @@ object Test { object M extends C // error: illegal inheritance: self type Test.M.type of object M$ does not conform to self type B of parent class C } - -trait X { self: Y => } // error: missing requirement: self type Y & X of trait X does not conform to self type Z of required trait Y -trait Y { self: Z => } -trait Z diff --git a/tests/pos/i16407.scala b/tests/pos/i16407.scala new file mode 100644 index 000000000000..8529f81a29af --- /dev/null +++ b/tests/pos/i16407.scala @@ -0,0 +1,7 @@ +trait X { //missing requirement: self type Z[?] & X of trait X does not conform to self type Z[X.this.A] of required trait Z + self: Z[_] => +} + +trait Z[A] extends X { + self: Z[A] => // comment this to compile successfully +} \ No newline at end of file diff --git a/tests/pos/open-selftype.scala b/tests/pos/open-selftype.scala new file mode 100644 index 000000000000..e834f405ec7f --- /dev/null +++ b/tests/pos/open-selftype.scala @@ -0,0 +1,8 @@ + +trait X { self: Y => } // error: missing requirement: self type Y & X of trait X does not conform to self type Z of required trait Y +trait Y { self: Z => } +trait Z + +package squants: + trait Quantity[A <: Quantity[A]] { self: A => } + trait TimeDerivative[A <: Quantity[A]] { self: Quantity[_] => } \ No newline at end of file From dd1e19861af72490ec457f5aca91625e753a1e16 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jan 2023 15:12:17 +0100 Subject: [PATCH 2/3] Add test and drop diagnostic output unless debugging --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 11 ++++------- tests/neg/i16407.check | 12 ++++++++++++ tests/neg/i16407.scala | 11 +++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i16407.check create mode 100644 tests/neg/i16407.scala diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 06b21bab6443..b34416a095db 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -610,16 +610,13 @@ object Denotations { */ def signature(sourceLanguage: SourceLanguage)(using Context): Signature = if (isType) Signature.NotAMethod // don't force info if this is a type denotation - else info match { + else info match case info: MethodOrPoly => try info.signature(sourceLanguage) - catch { // !!! DEBUG - case scala.util.control.NonFatal(ex) => - report.echo(s"cannot take signature of $info") - throw ex - } + catch case ex: Exception => + if ctx.debug then report.echo(s"cannot take signature of $info") + throw ex case _ => Signature.NotAMethod - } def derivedSingleDenotation(symbol: Symbol, info: Type, pre: Type = this.prefix, isRefinedMethod: Boolean = this.isRefinedMethod)(using Context): SingleDenotation = if ((symbol eq this.symbol) && (info eq this.info) && (pre eq this.prefix) && (isRefinedMethod == this.isRefinedMethod)) this diff --git a/tests/neg/i16407.check b/tests/neg/i16407.check new file mode 100644 index 000000000000..5c6bd19ca8c1 --- /dev/null +++ b/tests/neg/i16407.check @@ -0,0 +1,12 @@ +-- Error: tests/neg/i16407.scala:2:2 ----------------------------------------------------------------------------------- +2 | f(g()) // error // error + | ^ + | cannot resolve reference to type (X.this : Y & X).A + | the classfile defining the type might be missing from the classpath + | or the self type of (X.this : Y & X) might not contain all transitive dependencies +-- Error: tests/neg/i16407.scala:2:4 ----------------------------------------------------------------------------------- +2 | f(g()) // error // error + | ^ + | cannot resolve reference to type (X.this : Y & X).A + | the classfile defining the type might be missing from the classpath + | or the self type of (X.this : Y & X) might not contain all transitive dependencies diff --git a/tests/neg/i16407.scala b/tests/neg/i16407.scala new file mode 100644 index 000000000000..ff7192390eef --- /dev/null +++ b/tests/neg/i16407.scala @@ -0,0 +1,11 @@ +trait X { self: Y => + f(g()) // error // error +} +trait Y { self: Z => + type B = A + def f(a: B): Unit = () + def g(): A = ??? +} +trait Z { + type A +} From 689a9424949c447fa1964635a4797546183a9937 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 10 Jan 2023 15:48:14 +0100 Subject: [PATCH 3/3] Revert to old definition if isSelfSym --- .../dotty/tools/dotc/core/SymDenotations.scala | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 2b5c51188571..e267bc51758f 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -702,8 +702,21 @@ object SymDenotations { flagsUNSAFE.is(Provisional) // do not force the info to check the flag /** Is this the denotation of a self symbol of some class? - */ - final def isSelfSym(using Context): Boolean = is(SelfName) + * This is the case if one of two conditions holds: + * 1. It is the symbol referred to in the selfInfo part of the ClassInfo + * which is the type of this symbol's owner. + * 2. This symbol is owned by a class, it's selfInfo field refers to a type + * (indicating the self definition does not introduce a name), and the + * symbol's name is "_". + * TODO: Find a more robust way to characterize self symbols, maybe by + * spending a Flag on them? + */ + final def isSelfSym(using Context): Boolean = owner.infoOrCompleter match { + case ClassInfo(_, _, _, _, selfInfo) => + selfInfo == symbol || + selfInfo.isInstanceOf[Type] && name == nme.WILDCARD + case _ => false + } /** Is this definition contained in `boundary`? * Same as `ownersIterator contains boundary` but more efficient.