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

Also merge var with unchanged bounds. #46757

Merged
merged 2 commits into from
Sep 15, 2022
Merged

Conversation

N5N3
Copy link
Member

@N5N3 N5N3 commented Sep 14, 2022

I skipped it as I thought unchanged bounds means there's no restriction added on this var.
But examples from #46735 show that it should not be skiped as the current env might not be valid if the var's bounds get fixed (by another Unions decision.)

close #46735. Test added.

We should not skip it as the current `env` might not be valid if the var's bounds get fixed (by another Unions decision.)
@N5N3 N5N3 added types and dispatch Types, subtyping and method dispatch bugfix This change fixes an existing bug backport 1.8 Change should be backported to release-1.8 labels Sep 14, 2022
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

SGTM

src/subtype.c Outdated Show resolved Hide resolved
and remove the unneeded Null check.

Co-Authored-By: Jameson Nash <[email protected]>
@N5N3 N5N3 merged commit 6557542 into JuliaLang:master Sep 15, 2022
@N5N3 N5N3 deleted the more_env_merge branch September 15, 2022 11:44
@KristofferC KristofferC mentioned this pull request Sep 16, 2022
28 tasks
KristofferC pushed a commit that referenced this pull request Sep 16, 2022
* Also merge var with unchanged bounds.
The current `env` might not be valid if the var's bounds get fixed by another Unions decision.
* Replace `v->var->lb` with `simple_meet`
* Remove the unneeded NUll check.

Co-Authored-By: Jameson Nash <[email protected]>
(cherry picked from commit 6557542)
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Sep 30, 2022
aviatesk pushed a commit that referenced this pull request Dec 9, 2022
* Also merge var with unchanged bounds.
The current `env` might not be valid if the var's bounds get fixed by another Unions decision.
* Replace `v->var->lb` with `simple_meet`
* Remove the unneeded NUll check.

Co-Authored-By: Jameson Nash <[email protected]>
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.

Unreachable reached in AtomicStructures.jl on Julia 1.8.1, but not 1.8.0
3 participants