8000 Canonicalize capture variable subtype comparisons by bracevac · Pull Request #22299 · scala/scala3 · GitHub
[go: up one dir, main page]
Skip to content

Canonicalize capture variable subtype comparisons #22299

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Canonicalize capture variable subtype comparisons #22299

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a587261
Canonicalize capture variable subtype comparisons
bracevac Jan 21, 2025
24ddd74
Polishings
odersky Jan 22, 2025
c3fcd7c
Comments
bracevac Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Comments
  • Loading branch information
@bracevac
bracevac committed Jan 22, 2025
commit c3fcd7cd2772a6b75ae3bdb09c8a857efd66cf12
14 changes: 13 additions & 1 deletion compiler/src/dotty/tools/dotc/cc/CaptureRef.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,15 @@ trait CaptureRef extends TypeProxy, ValueType:
case info: SingletonCaptureRef => test(info)
case info: AndType => viaInfo(info.tp1)(test) || viaInfo(info.tp2)(test)
case info: OrType => viaInfo(info.tp1)(test) && viaInfo(info.tp2)(test)
case info @ CapturingType(_,_) if this.derivesFrom(defn.Caps_CapSet) => test(info)
case info @ CapturingType(_,_) if this.derivesFrom(defn.Caps_CapSet) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this condition check this instead of info?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case here does not look currect for me.viaInfo is used only if there is some equivalent relation from singleton types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tailored to the case that we want to check that a term variable reaches a capture variable (resp. the capture variable subsumes the term variable, so we check the this), because of the issue outlined in
#22299 (comment)
Not sure if this is best place to do it, but logically we have to chase along the infos of the context entries. The case y: TermRef if !y.isRootCapability => part will invoke this function as a last resort, and since we have to follow the info then, we might as well do it here.

Feel free to open a new PR if you see a better way to do it.

/*
If `this` is a capture set variable `C^`, then it is possible that it can be
reached from term variables in a reachability chain through the context.
For instance, in `def test[C^](src: Foo^{C^}) = { val x: Foo^{src} = src; val y: Foo^{x} = x; y }`
we expect that `C^` subsumes `x` and `y` in the body of the method
(cf. test case cc-poly-varargs.scala for a more involved example).
*/
test(info)
case _ => false

(this eq y)
Expand Down Expand Up @@ -150,6 +158,10 @@ trait CaptureRef extends TypeProxy, ValueType:
case TypeBounds(_, hi: CaptureRef) => this.subsumes(hi)
case _ => y.captureSetOfInfo.elems.forall(this.subsumes)
case CapturingType(parent, refs) if parent.derivesFrom(defn.Caps_CapSet) || this.derivesFrom(defn.Caps_CapSet) =>
/* The second condition in the guard is for `this` being a `CapSet^{a,b...}` and etablishing a
potential reachability chain through `y`'s capture to a binding with
`this`'s capture set (cf. `CapturingType` case in `def viaInfo` above for more context).
*/
refs.elems.forall(this.subsumes)
case _ => false
|| this.match
Expand Down
Loading
0