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

typeintersect: fix constraintkind for non-covariant var #50209

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

N5N3
Copy link
Member

@N5N3 N5N3 commented Jun 18, 2023

The pre-check for constraintkind failed to predict occurs_cov (!var_occurs_invariant doesn't means occurs_cov > 0). And false constraintkind might cause circular constraints during the env contraction.

This PR fixes it by adding missing check back.
MWE added.
close #50195.

@N5N3 N5N3 added the domain:types and dispatch Types, subtyping and method dispatch label Jun 18, 2023
src/subtype.c Show resolved Hide resolved
src/subtype.c Outdated
always_occurs_cov(ua->var->lb, var, 0) ||
always_occurs_cov(ua->var->ub, var, 0) ||

This comment was marked as outdated.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think I misunderstood the meaning of the always_occurs_cov to mean only_cov, which was not the intent. I guess this function is actually equivalent to Core.Compiler.constrains_param && !occurs_inv

A use as a lower bound does not create a constraint on the lower bound for var:

Suggested change
always_occurs_cov(ua->var->lb, var, 0) ||
always_occurs_cov(ua->var->ub, var, 0) ||
always_occurs_cov(ua->var->ub, var, 0) ||

Copy link
Member Author

@N5N3 N5N3 Jun 22, 2023

Choose a reason for hiding this comment

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

I guess they have some little difference. But the suggestions make sense (As their task looks similar).

always_occurs_cov should perform a conservative estimation on the occurs_cov after 1st intersection.
As we want to match the branch if (vb.occurs_cov && noinv) below.

var_occurs_inside is not a good template here:

  1. We only perform intersect_aside on ub.
  2. The union decision should not affect the conservativeness.

src/subtype.c Outdated Show resolved Hide resolved
@maleadt
Copy link
Member

maleadt commented Jun 21, 2023

Thanks! Confirmed that this makes the CUDA.jl hang disappear (i.e. not only the reduced MWE).

N5N3 and others added 3 commits June 22, 2023 21:06
`!var_occurs_invariant` doesn't inplies that `occurs_cov > 0`.
Adding the missing check.
Co-Authored-By: Jameson Nash <[email protected]>
@vtjnash vtjnash added the status:merge me PR is reviewed. Merge when all tests are passing label Jun 22, 2023
@IanButterworth IanButterworth merged commit a8d76c6 into JuliaLang:master Jun 22, 2023
2 checks passed
@IanButterworth IanButterworth removed the status:merge me PR is reviewed. Merge when all tests are passing label Jun 22, 2023
@N5N3 N5N3 deleted the intersect-fix branch June 23, 2023 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hang during type intersection
4 participants