Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
compiler:inference Type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants