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

don't warn about redefining methods in Main #19888

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jan 6, 2017

This PR closes #18725 by silencing the "Method definition overwritten" warnings when both the new and old methods are in Main.

The warnings are useful for modules, where overwritten methods are usually an error, but are incredibly annoying and confusing during interactive work in the REPL (or IJulia etc.). (Now that #265 is fixed, redefining functions is much less dangerous, so most of the concerns raised in #18725 should now be moot.)

With this PR:

julia> f(x) = 3
f (generic function with 1 method)

julia> f(x) = 12
f (generic function with 2 methods)

julia> f(4)
12

julia> module Foo
           f(x) = 3
           f(x) = 4
       end
WARNING: Method definition f(Any) in module Foo at REPL[7]:2 overwritten at REPL[7]:3.
Foo

@JeffBezanson
Copy link
Sponsor Member

Yes, with #265 fixed I'd say this is ok. Not sure about this, but maybe we should limit this to interactive functions, i.e. functions not in any file?

@stevengj
Copy link
Member Author

stevengj commented Jan 6, 2017

@JeffBezanson, people regularly include a file with function definitions repeatedly. (And in IJulia the input cell "In[xx]" is treated as the "filename", so that might be affected too.)

@KristofferC
Copy link
Sponsor Member

KristofferC commented Jan 6, 2017

Previous implementation and discussion at #18746 (also includes test)

@stevengj stevengj merged commit 8192aff into JuliaLang:master Jan 6, 2017
@stevengj stevengj deleted the overwrite_nowarn branch January 6, 2017 12:22
@tkelman
Copy link
Contributor

tkelman commented Jan 6, 2017

the version with a test was better

@stevengj
Copy link
Member Author

stevengj commented Jan 6, 2017

We don't normally test warnings. If we want to do that, we need a better test framework than setting up redirect_stdout manually every time

@tkelman
Copy link
Contributor

tkelman commented Jan 6, 2017

The work had been done manually for this case. There are some convenience macros in the Pkg tests that Carlo wrote that would be worth moving to a more general place and making more use of.

@timholy
Copy link
Sponsor Member

timholy commented Jan 6, 2017

Even better, we already have #18165. It's now very easy to do this correctly.

@stevengj
Copy link
Member Author

stevengj commented Jan 6, 2017

#19903 shows what I had in mind for a nicer test framework for warnings than manual redirect_stderr calls, and adds a test for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Julia 0.5 now complains about redefining methods
5 participants