-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refine rules for capture parameters and members #22000
base: main
Are you sure you want to change the base?
Refine rules for capture parameters and members #22000
Conversation
651b674
to
c933560
Compare
TODO: fix override check for type members during CC |
d016cce
to
a8c624e
Compare
|
||
|
||
|
||
race[Either[T1, T2], Cap](left, right) |
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 should not be necessary. I'll look into normalization and subtyping of captures.
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.
The subtyping Cap <:< CapSet^{Cap^}
should work. We will fix this in a different PR.
Co-authored-by: Oliver Bračevac <[email protected]>
008eb08
to
5a901f4
Compare
@bracevac This PR should be ready to review. The one error in |
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.
Otherwise LGTM
@@ -4057,8 +4057,11 @@ object Parsers { | |||
|| sourceVersion.isAtLeast(`3.6`) && in.isColon => | |||
makeTypeDef(typeAndCtxBounds(tname)) | |||
case _ => | |||
syntaxErrorOrIncomplete(ExpectedTypeBoundOrEquals(in.token)) | |||
return EmptyTree // return to avoid setting the span to EmptyTree | |||
if in.isIdent(nme.UPARROW) && Feature.ccEnabled then |
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 not backed by a comment or a syntax description. Do we want to accept
type C^
now? I suggest we wait with introducing this. The syntax might change. And for now, we can just write the expansion. So I would propose we drop this change for the time being.
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.
Sure
@@ -135,14 +142,26 @@ trait CaptureRef extends TypeProxy, ValueType: | |||
case _ => false | |||
|| viaInfo(y.info)(subsumingRefs(this, _)) | |||
case MaybeCapability(y1) => this.stripMaybe.subsumes(y1) | |||
case y: TypeRef if y.derivesFrom(defn.Caps_CapSet) => | |||
y.info match | |||
case TypeBounds(_, hi: CaptureRef) => this.subsumes(hi) |
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.
Can this case happen? I thought the upper bound has to be of the form CapSet^cs
? If it can happen, maybe add a comment explaining why.
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.
The upper and lower bounds don't have to be in the form of CapSet^{...}
. They can also be other capture set variables, which are bounded by CapSet
, like def test[X^, Y^, Z >: X <: Y]
. With this set up, both cases would happen during subsumes
.
case x: TypeRef if assumedContainsOf(x).contains(y) => true | ||
case x: TypeRef if x.derivesFrom(defn.Caps_CapSet) => | ||
x.info match | ||
case TypeBounds(lo: CaptureRef, _) => |
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.
Same as above; can this case happen?
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.
Might need a rebase to make the tests pass.
Currently blocked by error on main branch: #22058 |
This PR refines rules for capture set variables (parameters and members).
Fix #21999, #22005, #22030
Add requirements for capture set variables
When a capture set is encoded as a type, the type must refer to
CapSet
and bounded by>: CapSet <: CapSet^
.An unbounded capture parameter would be
C >: CapSet <: CapSet^
, which can be desugared fromC^
.We may consider the similar desugaring for type member in the future:
Then, constaints between capture variables become possible:
Update definitions in the library
caps.scala
, such that a type following the rule can be used inside a capture set.Add cases to handle
CapSet
insubsumes
Fix some issues related to overriding
When deciding whether a class has a non-trivial self type, we look at the underlying type without capture set.
[test_scala2_library_tasty]