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

Improve inference of call chains to TOML.print #1824

Merged
merged 1 commit into from
May 14, 2020
Merged

Improve inference of call chains to TOML.print #1824

merged 1 commit into from
May 14, 2020

Conversation

timholy
Copy link
Member

@timholy timholy commented May 13, 2020

This is a better fix for #1806, one that resolves the inference problems. A big part of the problem is the dreaded 15276.

Addresses concerns in JuliaLang/www.julialang.org#794 (comment).

This is a better fix for #1806, one that resolves the inference problems.
A big part of the problem is the dreaded 15276.
@KristofferC KristofferC merged commit 5fbcb0a into JuliaLang:master May 14, 2020
@@ -673,8 +673,10 @@ function bind_artifact!(artifacts_toml::String, name::String, hash::SHA1;
end

# Spit it out onto disk
open(artifacts_toml, "w") do io
TOML.print(io, artifact_dict, sorted=true)
let artifact_dict = artifact_dict
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised this is needed, there is no assignment to artifact_dict so shouldn't the frontend be able to figure this out?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a hundred percent certain but the codegen shows some try/catch stuff that appears to complicate the analysis. In practice, adding this suddenly gave much nicer codegen.

@timholy timholy deleted the teh/instability branch May 14, 2020 08:48
fredrikekre pushed a commit that referenced this pull request May 29, 2020
This is a better fix for #1806, one that resolves the inference problems.
A big part of the problem is the dreaded 15276.

(cherry picked from commit 5fbcb0a, PR #1824)
@timholy timholy added the latency label Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants