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

the "special-case inliners for known pure functions that compute types" are broken #18735

Closed
wongwill86 opened this issue Sep 29, 2016 · 2 comments
Labels
bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference

Comments

@wongwill86
Copy link

This can be reproduced with different versions of julia (.4 to .5) as well as using BaseTestDeprecated vs Base.Test.

Example below:

This is the module we are testing:

julia> module ToTest
       function get_type(s::String)
           println("get_type being called")
           if s != "UInt16"
               error("I don't know what $s is.")
           end
           return UInt16
       end
       function get_string(s::String)
           println("get_string being called")
           if s != "UInt16"
               error("I'm erroring for fun")
           end
           return s
       end
       end

This is the module that tests ToTest with old Base.Test aka BaseTestDeprecated

julia> module TesterBaseDeprecated
       import ToTest
       using BaseTestDeprecated
       const Test = BaseTestDeprecated
       function test_get_type(s::String)
           @test_throws Exception ToTest.get_type(s)
       end
       function test_get_string(s::String)
           @test_throws Exception ToTest.get_string(s)
       end
       end

This is the module that tests ToTest with new Base.Test

julia> module TesterBaseNext
       import ToTest
       using Base.Test
       function test_get_type(s::String)
           @test_throws Exception ToTest.get_type(s)
       end
       function test_get_string(s::String)
           @test_throws Exception ToTest.get_string(s)
       end
       end

In this example, ToTest.get_type(s::String) should throw an error if the s is not "UInt16"
BaseTestDeprecated works correctly by catching the error:

julia> TesterBaseDeprecated.test_get_type("should throw")
get_type being called
ErrorException("I don't know what should throw is.")

New Base.Test works incorrectly by not even running the function

julia> TesterBaseNext.test_get_type("should throw")
Test Failed
  Expression: ToTest.get_type(s)
    Expected: Exception
  No exception thrown
ERROR: There was an error during testing
 in record(::Base.Test.FallbackTestSet, ::Base.Test.Fail) at ./test.jl:397
 in do_test_throws(::Base.Test.Returned, ::Expr, ::Type{T}) at ./test.jl:329
 in test_get_type(::String) at ./REPL[18]:5

ToTest.test_get_string works correctly:

julia> TesterBaseNext.test_get_string("should throw")
get_string being called
Test Passed
  Expression: ToTest.get_string(s)
      Thrown: ErrorException

julia> TesterBaseDeprecated.test_get_string("should throw")
get_string being called
ErrorException("I'm erroring for fun")

Is there something wrong the way we are using @test_throws with the new Base.Test?

@tkelman tkelman added the testsystem The unit testing framework and Test stdlib label Sep 29, 2016
@TotalVerb
Copy link
Contributor

TotalVerb commented Oct 2, 2016

Smaller repro:

julia> @noinline f(n) = n ? error() : Int
f (generic function with 1 method)

julia> g() = Union{f(true)}
g (generic function with 1 method)

julia> g()
Int64

Some constructors like Nullable also do this.

@vtjnash vtjnash added bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference and removed testsystem The unit testing framework and Test stdlib labels Oct 2, 2016
@vtjnash
Copy link
Member

vtjnash commented Oct 2, 2016

This is the inlining bug noted at b3dbc05#commitcomment-17890328 (where f in the example here is apply_type)

@vtjnash vtjnash changed the title test_throws won't execute function that returns a type the "special-case inliners for known pure functions that compute types" are broken Oct 2, 2016
KristofferC added a commit that referenced this issue Jun 25, 2017
KristofferC added a commit that referenced this issue Jun 25, 2017
DrTodd13 pushed a commit to IntelLabs/julia that referenced this issue Jun 26, 2017
ararslan pushed a commit that referenced this issue Sep 11, 2017
Ref #22530
(cherry picked from commit 6218a1f)
ararslan pushed a commit that referenced this issue Sep 13, 2017
(cherry picked from commit 6218a1f)
vtjnash pushed a commit that referenced this issue Sep 14, 2017
(cherry picked from commit 6218a1f)
ararslan pushed a commit that referenced this issue Sep 15, 2017
(cherry picked from commit 6218a1f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference
Projects
None yet
Development

No branches or pull requests

4 participants