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

Fix inInference state corruption. #22313

Merged
merged 1 commit into from
Jun 10, 2017
Merged

Fix inInference state corruption. #22313

merged 1 commit into from
Jun 10, 2017

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Jun 9, 2017

This led to non-idempotent irgen.
Fixes #22290, caused by #21677.

@maleadt maleadt requested a review from vtjnash June 9, 2017 15:32
@@ -855,4 +862,4 @@ let f, m
Expr(:return, Expr(:call, Core._apply, :+, SSAValue(0)))
]
@test @inferred(f()) == 6
end
end
Copy link
Member

Choose a reason for hiding this comment

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

My bad, introduced when fixing conflicts

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

there's a fix needed to one of the staged tests too (to pass CI)

Copy link
Member

Choose a reason for hiding this comment

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

Not sure what you mean. Is the fix needed due to an issue with my conflict resolution?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

no, just a change to another number to make the CI test pass

@ararslan ararslan added the compiler:inference Type inference label Jun 9, 2017
This led to non-idempotent irgen.
@maleadt
Copy link
Member Author

maleadt commented Jun 10, 2017

Rebased and updated failing staged function test.

@@ -174,7 +174,7 @@ let gf_err, tsk = @async nothing # create a Task for yield to try to run
end
@test_throws ErrorException gf_err()
@test_throws ErrorException gf_err()
@test gf_err_ref[] == 3
@test gf_err_ref[] == 4
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this change?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The value is unimportant, it just tells how many times we attempted to and failed to run the generated function and is useful for being aware of when it changes

Copy link
Contributor

Choose a reason for hiding this comment

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

why does the number of times this attempted to and failed to run the generated function change then?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

We used to disable attempts to reinfer a function if it failed. This PR changes it to always attempt to reinfer a function.

@vtjnash vtjnash merged commit fccbb66 into master Jun 10, 2017
@vtjnash vtjnash deleted the tb/fix_22290 branch June 10, 2017 15:05
maleadt pushed a commit that referenced this pull request Oct 30, 2017
This led to non-idempotent irgen.

(cherry picked from commit f91d54f, ref #22313)
ararslan pushed a commit that referenced this pull request Oct 30, 2017
This led to non-idempotent irgen.

(cherry picked from commit f91d54f, ref #22313)
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