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

meet_tvars: return bound tvar if both inputs are bound. Fixes #17648 #18355

Merged
merged 1 commit into from
Sep 7, 2016

Conversation

timholy
Copy link
Member

@timholy timholy commented Sep 4, 2016

Untested locally, let's see what breaks...

@vtjnash
Copy link
Member

vtjnash commented Sep 5, 2016

It passed, so does that mean it should be merged now?

@timholy
Copy link
Member Author

timholy commented Sep 5, 2016

Hmm, this doesn't go deep enough: I still can't turn on precompilation for ImagesAxes.jl.

@timholy timholy changed the title Defer to the more specific bound typevar in intersection. Fixes #17648 meet_tvars: return bound tvar if both inputs are bound. Fixes #17648 Sep 5, 2016
@timholy
Copy link
Member Author

timholy commented Sep 5, 2016

OK, this version allows me to turn on precompilation for ImagesAxes. It passes tests locally, so assuming CI passes this version can be merged.

@vtjnash vtjnash merged commit f33d155 into master Sep 7, 2016
@vtjnash vtjnash deleted the teh/sym_tv branch September 7, 2016 03:26
@vtjnash
Copy link
Member

vtjnash commented Sep 7, 2016

Should we run PkgEval on this before backporting?

@tkelman
Copy link
Contributor

tkelman commented Sep 7, 2016

Already started what will hopefully become rc4 if it looks good. How critical is this?

@timholy
Copy link
Member Author

timholy commented Sep 7, 2016

The only known real-world bug this fixes is not being able to turn on precompilation in ImagesAxes.jl, so I'd rate it as non-critical. If this run works out, I'd recommend going with rc4 and waiting on this one until the next time you're backporting.

If rc4 needs more tweaks, then it would be nice to have.

@timholy
Copy link
Member Author

timholy commented Sep 7, 2016

Just thought of one more issue to consider, though: while I think it's unlikely, it's possible that there are packages relying on the current (broken) behavior. (This affects dispatch, so fixing the bug could conceivably change which functions get called.) If that proves to be true, there's always the possibility that fixing the bug in 0.5.1 would be breaking.

As long as you're aware of this risk and willing to accept the fact that we might need to encourage those packages to make fixes, I'd still say to go with rc4 as-is unless it requires additional PkgEval runs anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants