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 definition overwritten warning could be more helpful... #35140

Closed
ericphanson opened this issue Mar 17, 2020 · 1 comment · Fixed by #36609
Closed

Method definition overwritten warning could be more helpful... #35140

ericphanson opened this issue Mar 17, 2020 · 1 comment · Fixed by #36609

Comments

@ericphanson
Copy link
Contributor

...in the case that a file is included twice. In that case, it shows up as

WARNING: Method definition (::Type{PseudoArcLengthContinuation.DefaultLS})() in module PseudoArcLengthContinuation at /Users/rveltz/work/prog_gd/julia/dev/dev1/PseudoArcLengthContinuation.jl/src/LinearSolver.jl:62 overwritten at /Users/rveltz/work/prog_gd/julia/dev/dev1/PseudoArcLengthContinuation.jl/src/LinearSolver.jl:62.
  ** incremental compilation may be fatally broken for this module **

(example from Slack). This is actually really helpful if you know the only way a method at LinearSolver.jl:62 can overwrite a method at LinearSolver.jl:62 is if the code is being evaluated twice, i.e., a duplicated include statement, but is pretty confusing if you don't realize that is possiblity. I propose adding a check to see if the file path and line number is the same for the two methods, and in that case changing the warning to something like

WARNING: Method definition (::Type{PseudoArcLengthContinuation.DefaultLS})() in module PseudoArcLengthContinuation at /Users/rveltz/work/prog_gd/julia/dev/dev1/PseudoArcLengthContinuation.jl/src/LinearSolver.jl:62 evaluated twice. Check for duplicate `include` statements.
  ** incremental compilation may be fatally broken for this module **

The warning is generated here

julia/src/gf.c

Lines 1372 to 1387 in 3b53f54

if (jl_options.warn_overwrite == JL_OPTIONS_WARN_OVERWRITE_ON) {
jl_method_t *method = (jl_method_t*)newentry->func.method;
jl_module_t *newmod = method->module;
jl_module_t *oldmod = oldvalue->module;
JL_STREAM *s = JL_STDERR;
jl_printf(s, "WARNING: Method definition ");
jl_static_show_func_sig(s, (jl_value_t*)newentry->sig);
jl_printf(s, " in module %s", jl_symbol_name(oldmod->name));
print_func_loc(s, oldvalue);
jl_printf(s, " overwritten");
if (oldmod != newmod)
jl_printf(s, " in module %s", jl_symbol_name(newmod->name));
print_func_loc(s, method);
jl_printf(s, ".\n");
jl_uv_flush(s);
}
but I don't really know C, so I filed this issue instead of making a PR.

@vtjnash
Copy link
Member

vtjnash commented Jul 10, 2020

The risk here (with the proposed change) is that having f() = 1; f() = 2 on a line would print that same message. I'll make a PR with something similar however

vtjnash added a commit that referenced this issue Jul 10, 2020
Always show the warning for anonymous functions, but update the verbiage
to give additional information. This warning can still be avoided by
explicitly calling delete_method first.

fixes #32635
fixes #35140
refs #15602
vtjnash added a commit that referenced this issue Jul 10, 2020
Always show the warning for anonymous functions, but update the verbiage
to give additional information. This warning can still be avoided by
explicitly calling delete_method first.

fixes #32635
fixes #35140
refs #15602
StefanKarpinski pushed a commit that referenced this issue Jul 10, 2020
Always show the warning for anonymous functions, but update the verbiage
to give additional information. This warning can still be avoided by
explicitly calling delete_method first.

fixes #32635
fixes #35140
refs #15602
simeonschaub pushed a commit to simeonschaub/julia that referenced this issue Aug 11, 2020
Always show the warning for anonymous functions, but update the verbiage
to give additional information. This warning can still be avoided by
explicitly calling delete_method first.

fixes JuliaLang#32635
fixes JuliaLang#35140
refs JuliaLang#15602
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 a pull request may close this issue.

2 participants