Skip to content

Conversation

@TotalVerb
Copy link
Contributor

Never inline as constant anything inferred as Type{Tuple{Vararg}}, as the actual type could be less specific. Works around the inference part of #18450.

If this is desired, it might be a good idea to run benchmarks in case this pessimizes something.

@TotalVerb
Copy link
Contributor Author

#18457 will likely make this obsolete.

@kshyatt kshyatt added the compiler:inference Type inference label Sep 13, 2016
@vtjnash vtjnash merged commit f30c949 into JuliaLang:master Sep 16, 2016
@vtjnash
Copy link
Member

vtjnash commented Sep 16, 2016

While it looks like #18457 is fixing this, it looks feasible to backport this to v0.5 to work around the error there. And it looks like a nice code refactoring anyways.

@TotalVerb
Copy link
Contributor Author

Sure. To backport this, I should just cherry-pick the commit and submit a PR against release-0.5?

@tkelman
Copy link
Contributor

tkelman commented Sep 16, 2016

the process is listed in #17418 but depending how critical this is it should probably wait until 0.5.1

tkelman referenced this pull request Sep 17, 2016
@TotalVerb
Copy link
Contributor Author

TotalVerb commented Sep 18, 2016

This might have accidentally pessimized inlining of Type{Tuple}, which I forgot was a supertype of Tuple{Vararg}. Not confirmed though. Will try to confirm and come up with a fix shortly if that's the case. @tkelman @Sacha0

@KristofferC
Copy link
Member

Should definitely run benchmarks before merging inference stuff.

@tkelman
Copy link
Contributor

tkelman commented Sep 18, 2016

Should definitely run benchmarks before merging inference stuff anything nontrivial.

FTFY

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:inference Type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants