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

Work around inference bug with Vararg #18467

Merged
merged 1 commit into from
Sep 16, 2016

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