Fix/#692 transitive selfrefs #702
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| private def otherReason(pre: Type)(implicit ctx: Context): String = pre match { | ||
| case pre: ThisType if pre.givenSelfType.exists => | ||
| i"\nor the self type of $pre might not contain all transitive dependencies" |
There was a problem hiding this comment.
isn't \n here an escaped "end of line" character?
There was a problem hiding this comment.
Ah, you're right, I misread that.
|
LGTM, needs rebasing. |
In the type `(A & B)(C.this)`, the first parens were missing, so the type displayed as A & B(C.this), which is confusing.
A TypeRef can have be unresolved, either because it refers to something that's missing from the classpath or because of transitive self type references. Instead of crashing in sigName, we now report the error. Achieved by defining a new exception type, MissingType, which derives from TypeError. This catches t7933.scala, now integrated in the neg/selfreq.scala. The problem there was a reference to AbsSettings, which was not a member of StandardScalaSettings.this, but was a member of the required type of AbsSettings, which itself appeared in the required type of StandardScalaSettings. We will outlaw in the next commit such transitive required references. Also collapsed TypeError and FatalTypeError. It was a misnomer anyway. Fatal were those type errors that were caught and reported! Open: Where else we should check for unresolved NamedTypes.
What is checked: A self type T is a subtype of all selftypes of classes refernced by T. That is, a self type has to subsume all self types of its required type. Ot, otherwise said, requirements must be closed; you cannot discover new ones in following them.
odersky
force-pushed
the
fix/#692-transitive-selfrefs
branch
from
40cd899 to
1754995
Compare
|
Rebased to master |
|
Closes #690 |
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 10, 2023
scala#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 scala#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.
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 10, 2023
scala#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 scala#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.
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 11, 2023
scala#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 scala#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.
odersky
added a commit
to dotty-staging/dotty
that referenced
this pull request
Jan 11, 2023
scala#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 scala#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.
odersky
added a commit
that referenced
this pull request
Jan 12, 2023
#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. Fixes #16407
little-inferno
pushed a commit
to little-inferno/dotty
that referenced
this pull request
Jan 25, 2023
scala#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 scala#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was a longer thread than I expected. Review by @DarkDimius