Skip to content
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

fix #30394, an unsoundness in ml_matches #30396

Merged
merged 1 commit into from
Dec 17, 2018
Merged

fix #30394, an unsoundness in ml_matches #30396

merged 1 commit into from
Dec 17, 2018

Conversation

JeffBezanson
Copy link
Sponsor Member

This fixes a corner case where a bug is caused, counter-intuitively, by an over-estimated intersection.

Here's the setup:
We have method signatures A and B, with A<B (A is a strict subtype). We have a dispatch tuple X, where X<:B and !(X<:A). However, intersection returns X for intersect(X,A).

Since there appears to be a match with A and A<B, ml_matches skips the match with B. The fix just requires dispatch tuples to be a subtype of a signature in order to match at all. The unsoundness itself is actually in the logic just below the new code which sets skip = 1.

This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.
@JeffBezanson JeffBezanson added types and dispatch Types, subtyping and method dispatch bugfix This change fixes an existing bug backport pending 1.0 labels Dec 14, 2018
@JeffBezanson
Copy link
Sponsor Member Author

JeffBezanson commented Dec 15, 2018

I'll note that the logic in question:

                    jl_subtype(closure->match.ti, prior_ti)) {
                skip = 1;

can still be unsound: prior_ti is a type intersection result, and the bigger it is the more likely this is to be true and so the more likely we are to incorrectly skip a match. However I think other variations of this bug are a bit less likely to occur, and also seem more expensive to prevent, so I'll leave it for future work.

@JeffBezanson JeffBezanson merged commit b167bc2 into master Dec 17, 2018
@JeffBezanson JeffBezanson deleted the jb/fix30394 branch December 17, 2018 16:39
KristofferC pushed a commit that referenced this pull request Dec 20, 2018
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
@KristofferC KristofferC mentioned this pull request Dec 20, 2018
52 tasks
@KristofferC KristofferC mentioned this pull request Dec 30, 2018
53 tasks
KristofferC pushed a commit that referenced this pull request Dec 30, 2018
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
@StefanKarpinski StefanKarpinski added triage This should be discussed on a triage call and removed triage This should be discussed on a triage call labels Jan 31, 2019
@JeffBezanson JeffBezanson added backport 1.0 and removed triage This should be discussed on a triage call triage backport pending 1.0 labels Jan 31, 2019
@KristofferC KristofferC mentioned this pull request Feb 4, 2019
39 tasks
KristofferC pushed a commit that referenced this pull request Feb 4, 2019
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
KristofferC pushed a commit that referenced this pull request Feb 11, 2019
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
KristofferC pushed a commit that referenced this pull request Apr 20, 2019
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
This fixes a corner case where a bug is caused, counter-intuitively,
by an over-estimated intersection.
We have method signatures A and B, with A<B (A is a strict subtype).
We have a dispatch tuple X, where X<:B and !(X<:A).
However, intersection returns X for intersect(X,A). Since there
appears to be a match there and A<B, ml_matches skips the match with B.
The fix just requires dispatch tuples to be a subtype of a signature
in order to match at all.

(cherry picked from commit b167bc2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants