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

Method overwriting by an ambiguity should also invalidate the method cache #21965

Merged
merged 1 commit into from
May 29, 2017

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented May 19, 2017

fix #21963

DO NOT MERGE (until JuliaStrings/LegacyStrings.jl#8 is fixed)

@martinholters
Copy link
Member

With JuliaStrings/LegacyStrings.jl#8 fixed (and a new LegacyStrings version tagged),
@nanosoldier runbenchmarks(ALL, vs = ":master") should succeed now - let's see...

@KristofferC
Copy link
Member

Should be packported?

@vchuravy vchuravy added backport pending 0.6 bugfix This change fixes an existing bug labels May 29, 2017
@martinholters
Copy link
Member

As long as we don't backport #21892 (and I guess we won't), there AFAICT is no need to backport this. The fact that a method is still callable despite there being an ambiguity should rarely be a problem. OTOH, this can be a breaking change by uncovering bugs (as it was for LegacyStrings), which makes it an unlikely backport candidate.

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@martinholters
Copy link
Member

The performance regressions should all be noise, and I guess the travis OSX failure is unrelated, but I don't really know what happened there.

@vtjnash vtjnash merged commit 67b9b3e into master May 29, 2017
@vtjnash vtjnash deleted the jn/21963 branch May 29, 2017 15:06
ararslan pushed a commit that referenced this pull request Sep 11, 2017
Method overwriting by an ambiguity should also invalidate the method cache

Ref #21965
(cherry picked from commit 67b9b3e)
ararslan pushed a commit that referenced this pull request Sep 13, 2017
vtjnash added a commit that referenced this pull request Sep 14, 2017
ararslan pushed a commit that referenced this pull request Sep 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ambiguous method callable
6 participants