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

Misc tests for issues fixed by jb/subtype #20097

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

mauro3
Copy link
Contributor

@mauro3 mauro3 commented Jan 17, 2017

Added misc tests referenced in #19998

Added tests for issues #12580, #18348, #13165, #11803, #12721. I placed those in inference.jl, is that correct or should I move them?

Enabled tests for #11840 (cc: @vtjnash)

@vtjnash
Copy link
Member

vtjnash commented Jan 17, 2017

Unless they were related to an inference optimization, I think most of these should end up in subtype.jl

#11840 isn't fixed. If that test is passing, it just means I need to update it

@tkelman tkelman added the test This change adds or pertains to unit tests label Jan 17, 2017
@kshyatt kshyatt added the types and dispatch Types, subtyping and method dispatch label Jan 18, 2017
@vtjnash vtjnash mentioned this pull request Jan 18, 2017
53 tasks
@test TypeVar(:T, Union{Int,Float64}) <: Union{Int,Float64}

# Issue #12721
f12721{T<:Type{Int}}(::T) = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

semicolon not necessary

@mauro3
Copy link
Contributor Author

mauro3 commented Jan 18, 2017

I'll move the tests to subtype.jl. Unless I hear differently, I'll leave the 11840 tests as they are now but add the FIXME comment again. @vtjnash if you got an updated test, I could include it too.

@mauro3 mauro3 force-pushed the m3/misc-jb-subtype-tests branch from c2f5a97 to c2a1fba Compare January 18, 2017 08:10
@mauro3
Copy link
Contributor Author

mauro3 commented Jan 18, 2017

Updated. @vtjnash should give his "ok" before this gets merged.

@vchuravy vchuravy requested a review from vtjnash January 18, 2017 08:16
@test Hermitian{Complex{Float64},Matrix{Complex{Float64}}} <: LinAlg.RealHermSymComplexHerm

# Issue #11803
@test TypeVar(:T, Int) <: Int
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this test. Manually constructing a TypeVar is not really a supposed operation. I wonder if it would be fair to say that:
(Tuple{T} where T<:Union{Int,Float64}) <: Tuple{Union{Int64,Float64}} tests the same thing. @JeffBezanson?

Copy link
Member

Choose a reason for hiding this comment

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

I agree this test is invalid. Ideally it would be an error to pass anything with free type variables to <:. This file has plenty of tests that cover this sort of thing.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's just delete this test then.

@mauro3 mauro3 force-pushed the m3/misc-jb-subtype-tests branch from c2a1fba to 42ca9d1 Compare January 20, 2017 07:32
Added tests for issues JuliaLang#12580, JuliaLang#18348, JuliaLang#13165, JuliaLang#12721

For JuliaLang#11803 it was decidided that no tests are needed.

Enabled extra tests for JuliaLang#11840, however, that isssue is not resolved
yet but needs new tests triggering it.
@mauro3 mauro3 force-pushed the m3/misc-jb-subtype-tests branch from 42ca9d1 to 2cab8c0 Compare January 20, 2017 07:35
@Keno Keno merged commit fc4dc29 into JuliaLang:master Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test This change adds or pertains to unit tests types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants