-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Pre SIP: Improvements to Modularity #18958
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
Conversation
These are variations of a Scala Users discussion The critical part is shown in parsercombinators-expanded.scala. definition of def combine[A, B](
_f: Combinator[A],
_s: Combinator[B] { type Context = _f.Context}
): combine[A, B] {
type Context = _f.Context
type Element = (_f.Element, _s.Element)
} = new combine[A, B](_f, _s).asInstanceOf
// cast is needed since the type of new combine[A, B](_f, _s)
// drops the required refinement. We'd need to appy the scheme for instantiating dependent methods also to class instance creations. |
02eb2f7
to
0f4aba8
Compare
tracked
modifier to express class parameter dependencies
I now see that #3936 is very similar to this PR (and might also solve some problems wrt inheritance in this PR). Definitely worth following up. Maybe we should re-open #3936? /cc @liufengyun @julienrf |
Indeed it's similar. Glad to see there's real requirement from real-world programming. This PR is much more polished than #3936, it makes sense to keep the current PR. Some of the tests in #3936 can be ported here. BTW, I remember @mbovel is working on something related. |
I was particularly interested in the way #3936 handles base types. This PR has nothing in that department. Maybe that's needed to fix the remaining failures in neg/i3964.scala? |
I forgot exactly why the changes in handling base types, but it seems to be motivated by the last test case there. However, I'm not sure if the changes make sense, as I was not very familiar with the base type computation at the time. |
Some more info: Making all explicit vals tracked causes a failing bootstrap and ~50 failing tests. Particular issues:
So, not sure this idea will fly. |
I was wondering will the errors go away if
It seems this will go away if
This might go way as well with explicit
|
@liufengyun Yes, I agree, explicit |
Another thing that currently does not work is this test from object Test3:
trait Vec(tracked val size: Int)
class Vec8 extends Vec(8):
val s: 8 = size // error, but should work The problem is we currently keep track of refinements in constructors and therefore in constructed values. But we don't do the same thing inside classes. Inside classes we could experiment with
|
I was thinking whether it is possible to handle it in base type computation logic? Conceptually, it seems to be the same problem as refining types of members with regards to type parameters and prefixes. |
That's an interesting idea, but I am not sure about how this would work in detail? |
I don't think Also, |
I was thinking that in the following example the parent call trait Vec(tracked val size: Int)
class Vec8 extends Vec(8):
val s: 8 = size // error, but should work My knowledge is limited regarding the base type computation, just an intuition, so not sure if that would work. |
Here are a few notes, most of them coming from discussions we had in person. Do we need inheritance for classes with
|
30c78c1
to
afefb9e
Compare
This PR now bundles three improvements to modularity, all available under the
The doc page for these changes is The first two commits come from #19392 |
afefb9e
to
7ba2303
Compare
tracked
modifier to express class parameter dependencies
How about using class D(dep x: C) { type T = x.T } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice generalization to parent refinements 🎉
@@ -443,13 +443,13 @@ object desugar { | |||
private def toDefParam(tparam: TypeDef, keepAnnotations: Boolean): TypeDef = { | |||
var mods = tparam.rawMods | |||
if (!keepAnnotations) mods = mods.withAnnotations(Nil) | |||
tparam.withMods(mods & (EmptyFlags | Sealed) | Param) | |||
tparam.withMods(mods & EmptyFlags | Param) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is equivalent to just Param
?
@@ -1049,7 +1050,7 @@ class TreeUnpickler(reader: TastyReader, | |||
} | |||
val parentReader = fork | |||
val parents = readParents(withArgs = false)(using parentCtx) | |||
val parentTypes = parents.map(_.tpe.dealias) | |||
val parentTypes = parents.map(_.tpe.dealiasKeepAnnots.separateRefinements(cls, null)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compare this with pickling, it seems we loose symmetry?
@@ -62,7 +62,7 @@ object Parsers { | |||
case ExtensionFollow // extension clause, following extension parameter | |||
|
|||
def isClass = // owner is a class | |||
this == Class || this == CaseClass | |||
this == Class || this == CaseClass || this == Given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider change the method name or add a comment to avoid misuse --- it's not obvious for Given
.
|| mbr.is(Tracked) | ||
// Tracked members correspond to existing val parameters, so they don't | ||
// count as deferred. The val parameter could not implement the tracked | ||
// refinement since it usually has a wider type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminds me of the erased
flag. Might be possible to generalize in the future.
def m(): 22 | ||
|
||
class D extends R: | ||
def next(): D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should class C
and D
be abstract
?
|
Why not
|
|
Needs minor release since it introduces a new Tasty tag. |
Another possible keyword here would be |
This feature doesn't solve most of the problems the Precise Type SIP targeted so I don't see what is so |
My other comment was about the need to write both |
The rules for export forwarders are changed as follows. Previously, all export forwarders were declared `final`. Now, only term members are declared `final`. Type aliases left aside. This makes it possible to export the same type member into several traits and then mix these traits in the same class. `typeclass-aggregates.scala` shows why this is essential to be able to combine multiple givens with type members. The change does not lose safety since different type aliases would in any case lead to uninstantiatable classes.
Refinements of a class parent are added as synthetic members to the inheriting class. # Conflicts: # compiler/src/dotty/tools/dotc/config/ScalaSettings.scala # compiler/src/dotty/tools/dotc/typer/Implicits.scala # compiler/src/dotty/tools/dotc/typer/Namer.scala # tests/pos/typeclass-aggregates.scala
For a tracked class parameter we add a refinement in the constructor type that the class member is the same as the parameter. E.g. ```scala class C { type T } class D(tracked val x: C) { type T = x.T } ``` This will generate the constructor type: ```scala (x1: C): D { val x: x1.type } ``` Without `tracked` the refinement would not be added. This can solve several problems with dependent class types where previously we lost track of type dependencies.
d5dcbe0
to
adff7dc
Compare
This is part of #20061 |
For a tracked class parameter we add a refinement in the constructor type that
the class member is the same as the parameter. E.g.
This will generate the constructor type:
Without
tracked
the refinement would not be added. This can solve several problems with dependent classtypes where previously we lost track of type dependencies.
In particular it addresses the parser combinator test cases that were prompted by this discussion in Scala Users: https://users.scala-lang.org/t/create-an-instance-of-a-type-class-with-methods-depending-on-type-members/9613/4.
This PR is essentially a feasibility study. It shows that we can get better tracking of types through constructors without breaking anything.
But I am not sure yet a new modifier
tracked
is the best way to achieve this. Ideally, we would detect when a class uses a class parameter as a path in a type, and refine those parameters only. The problem is that we don't know this in general when we compute the type of a constructor application, since types of class members are computed lazily on first access.Example:
We need to typecheck the RHS of
f
inD
to figure out thatf
's result typex.T
refers to the parameterx
, so thatx
needs a refinement. So computing precise info of what needs to be refined is infeasible.Some of the other options are:
tracked
to indicate refinement. To make this more robust we could make it an error if a type in a public signature refers to an untracked class parameter. So in the example above, we could enforce that classD
is declared asclass D(tracked x: A)
.val
s tracked, and/or make all parameters ingiven ... with
clauses tracked.tracked
for parameters that appear "obviously" in a path. E.g. there is a path with the name of the parameter in an explicitly given signature in the class. Provide a fallback such as atracked
modifier or an annotation for parameters that are not obviously in a path. The goal of this compared to (1) is to be less annoying. Why insist on explicittracked
if it is clear from the source that a parameter needs to be refined? But I don't like the fragility of this scheme.