-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Inferring tracked #21628
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
Inferring tracked #21628
Conversation
d1562cb to
65a5ddd
Compare
|
Test logs for after enabling a subset of |
5b0c484 to
8073932
Compare
1c1e7c0 to
8fded39
Compare
8fded39 to
0ce9bab
Compare
…ure" This reverts commit 1e2c226.
0ce9bab to
7442d5a
Compare
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.
I only have few comments about cosmetic and naming.
Otherwise looks very good to me; the diff is small and the changes are clear.
The heuristics for determining if tracked should be added look good to me at first sight, but I haven't experimented extensively with it.
Infer `tracked` for parameters that are referenced in the public
signatures of the defining class.
e.g.
```scala 3
class OrdSet(val ord: Ordering) {
type Set = List[ord.T]
def empty: Set = Nil
implicit class helper(s: Set) {
def add(x: ord.T): Set = x :: remove(x)
def remove(x: ord.T): Set = s.filter(e => ord.compare(x, e) != 0)
def member(x: ord.T): Boolean = s.exists(e => ord.compare(x, e) == 0)
}
}
```
In the example above, `ord` is referenced in the signatures of the
public members of `OrdSet`, so a `tracked` modifier will be inserted
automatically.
Aldo generalize the condition for infering tracked for context bounds.
Explicit `using val` witnesses will now also be `tracked` by default.
This implementation should be safe with regards to not introducing
spurious cyclic reference errors.
Current limitations (I'll create separate issues for them, once this is
merged):
- Inferring `tracked` for given classes is done after the desugaring to
class + def, so the def doesn't know about `tracked` being set on the
original constructor parameter. This might be worked around by watching
the original symbol or adding an attachment pointer to the implicit
wrapper.
```scala 3
given mInst: (c: C) => M:
def foo: c.T = c.foo
```
- Passing parameters as an **inferred** `tracked` arguments in parents
doesn't work, since forcing a parent (term) isn't safe.
This can be replaced with a lint that is checked after Namer.
Infer
trackedfor parameters that are referenced in the public signatures of the defining class.e.g.
In the example above,
ordis referenced in the signatures of the public members ofOrdSet, so atrackedmodifier will be inserted automatically.Aldo generalize the condition for infering tracked for context bounds. Explicit
using valwitnesses will now also betrackedby default.This implementation should be safe with regards to not introducing spurious cyclic reference errors.
Current limitations (I'll create separate issues for them, once this is merged):
trackedfor given classes is done after the desugaring to class + def, so the def doesn't know abouttrackedbeing set on the original constructor parameter. This might be worked around by watching the original symbol or adding an attachment pointer to the implicit wrapper.trackedarguments in parents doesn't work, since forcing a parent (term) isn't safe.This can be replaced with a lint that is checked after Namer.