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

Use TypeVars in limit_type_depth in Unions in invariant position #21192

Merged
merged 2 commits into from
Mar 28, 2017

Conversation

martinholters
Copy link
Member

@martinholters martinholters commented Mar 28, 2017

Ref #20626 (comment) and https://discourse.julialang.org/t/strange-failure-in-v0-6/2911

Unfortunately, types were this make a difference seem to be the same where #21191 is hit, so until #21191 is fixed, this just changes the error produced...

For `isa(a, Const) && !isa(b, Const)`, `⊑` assumed `b` to be a type,
leading to an error if `b` was a `PartialTypeVar`. Fixed by using
`widenconst(b)`.
Using `Any` for depth-limited `Union` members is only correct in
covariant position, otherwise a `TypeVar` has be be introduced.
@JeffBezanson JeffBezanson merged commit f7d7665 into master Mar 28, 2017
@martinholters martinholters deleted the mh/fix20626 branch March 28, 2017 15:51
@martinholters martinholters added the needs tests Unit tests are required for this change label Mar 29, 2017
@martinholters
Copy link
Member Author

There is a disabled test which needs to be enabled when #21191 is fixed.

@pabloferz
Copy link
Contributor

There is a disabled test which needs to be enabled when #21191 is fixed.

I belive @test_broken is useful for this, as it would error during testing indicating that is no longer broken when fixed.

@KristofferC
Copy link
Member

Not if it crashes the system though, I presume?

@pabloferz
Copy link
Contributor

Ah right, from #21191 it seems that this would segfault.

@martinholters
Copy link
Member Author

Indeed, or rather hit an assertion, but to the same effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference needs tests Unit tests are required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants