Skip to content

Commit

Permalink
Consolitate redefined the method warning
Browse files Browse the repository at this point in the history
Currently redefining a method doesn't emit one but two warnings.
One at the location of the new method, and one at the location of
the old method.

I believe this is not ideal because when collecting warnings via
a custom `Warning.warn`, it has to be pieced together. It's even
more tricky because the second part may or may not be emitted
depending on whether the original method has an associated ISeq.

I think it's much better to emit a single warning with all the
information in one go.
  • Loading branch information
byroot committed Apr 23, 2024
1 parent a534358 commit fff2284
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions vm_method.c
Expand Up @@ -1015,7 +1015,6 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
old_def->type != VM_METHOD_TYPE_ALIAS) {
const rb_iseq_t *iseq = 0;

rb_warning("method redefined; discarding old %"PRIsVALUE, rb_id2str(mid));
switch (old_def->type) {
case VM_METHOD_TYPE_ISEQ:
iseq = def_iseq_ptr(old_def);
Expand All @@ -1027,10 +1026,16 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
break;
}
if (iseq) {
rb_compile_warning(RSTRING_PTR(rb_iseq_path(iseq)),
ISEQ_BODY(iseq)->location.first_lineno,
"previous definition of %"PRIsVALUE" was here",
rb_id2str(old_def->original_id));
rb_warning(
"method redefined; discarding old %"PRIsVALUE"\n%s:%d: warning: previous definition of %"PRIsVALUE" was here",
rb_id2str(mid),
RSTRING_PTR(rb_iseq_path(iseq)),
ISEQ_BODY(iseq)->location.first_lineno,
rb_id2str(old_def->original_id)
);
}
else {
rb_warning("method redefined; discarding old %"PRIsVALUE, rb_id2str(mid));
}
}
}
Expand Down

0 comments on commit fff2284

Please sign in to comment.