Skip to content

Don't cache inference results if not optimizing.#18888

Merged
maleadt merged 2 commits intomasterfrom
tb/uncached_reflection
Oct 13, 2016
Merged

Don't cache inference results if not optimizing.#18888
maleadt merged 2 commits intomasterfrom
tb/uncached_reflection

Conversation

@maleadt
Copy link
Copy Markdown
Member

@maleadt maleadt commented Oct 12, 2016

The logic had been switched since #18591:

@@ -575,14 +575,10 @@ function code_typed(f::ANY, types::ANY=Tuple; optimize=true)
     end
     types = to_tuple_type(types)
     asts = []
-    for x in _methods(f,types,-1)
+    for x in _methods(f, types, -1)
         meth = func_for_method_checked(x[3], types)
-        if optimize
-            (code, ty, inf) = Core.Inference.typeinf(meth, x[1], x[2], true)
-        else
-            (code, ty, inf) = Core.Inference.typeinf_uncached(meth, x[1], x[2], optimize=false)
-        end
-        inf || error("inference not successful") # Inference disabled
+        (code, ty) = Core.Inference.typeinf_code(meth, x[1], x[2], optimize, !optimize)
+        code === nothing && error("inference not successful") # Inference disabled?
         push!(asts, uncompressed_ast(meth, code) => ty)
     end
     return asts

I have a test, but it still fails after this fix (works on 0.5)...

fXXX() = return nothing
m = first(methods(fXXX, Tuple{}))
@test m.specializations == nothing

code_typed(fXXX, Tuple{}; optimize=false)
@test m.specializations == nothing

code_typed(fXXX, Tuple{}; optimize=true)
@test m.specializations != nothing

Has the behavior changed, or is there another caching issue out there? cc @vtjnash

The logic had been switched since #18591.
@maleadt maleadt added bugfix This change fixes an existing bug compiler:inference Type inference labels Oct 12, 2016
@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Oct 12, 2016

The code_typed still must create the specializations entry. It just shouldn't store the type inference result there.

@maleadt
Copy link
Copy Markdown
Member Author

maleadt commented Oct 12, 2016

OK, that has changed since 0.5 then. Added a test.

@maleadt maleadt merged commit 6839451 into master Oct 13, 2016
@vchuravy vchuravy deleted the tb/uncached_reflection branch October 13, 2016 00:17
let
f18888() = return nothing
m = first(methods(f18888, Tuple{}))
@test m.specializations == nothing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

comparisons to nothing should use === and !==

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 compiler:inference Type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants