Bug #5015
closedmethod_added" is called in addition to "method_undefined
Description
When a method is undefined via "undef_method :any_method", the following methods are called back:
- method_added
- method_undefined
The expected behaviour is that only "method_undefined" is called (similar to "remove_method").
class String
def self.method_added(id)
puts "callback: added #{id.id2name}"
end
def self.method_undefined(id)
puts "callback: undefined #{id.id2name}"
end
def self.method_removed(id)
puts "callback: removed #{id.id2name}"
end
end
puts "\nundef method (buildin)"
class String
undef_method :capitalize
end
puts "\ndefine method"
class String
def any_method(*args)
nil
end
end
puts "\nundef method"
class String
undef_method :any_method
end
puts "\ndefine method"
class String
def any_method(*args)
nil
end
end
puts "\nremove method"
class String
remove_method :any_method
end
OUTPUT:
undef method (buildin)
callback: added capitalize
callback: undefined capitalize
define method
callback: added any_method
undef method
callback: added any_method
callback: undefined any_method
define method
callback: added any_method
remove method
callback: removed any_method
Files
Updated by judofyr (Magnus Holm) over 13 years ago
But undef_method does actually add a new method: it adds a method
which prevents the superclass' method being called:
class Foo < String; def +() end; undef_method :+ end
Foo.new + "test" # => raises error
class Bar < String; def +() end; remove_method :+ end
Bar.new + "test" # => calls String#+
Seems consistant to me.
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
Magnus Holm wrote:
But undef_method does actually add a new method: it adds a method
which prevents the superclass' method being called
[...]
Whatever "undef_method" does, is an internal implementation detail, which should not call "method_added".
undef method
callback: added any_method # false callback, "any_method" was not added by user
callback: undefined any_method # correct callback, "any_method" was undefined by user
Updated by wishdev (John Higgins) over 13 years ago
Lazaridis Ilias wrote:
Magnus Holm wrote:
But undef_method does actually add a new method: it adds a method
which prevents the superclass' method being called
[...]Whatever "undef_method" does, is an internal implementation detail, which should not call "method_added".
What rule exactly says that internal implementations can't fire callbacks? You are specifically interested in internal implementation issues when you are interested in things like "method_added" - there is always a price to peeking under the hood.
undef method
callback: added any_method # false callback, "any_method" was not added by user
Where exactly is the rule that a method needs to be added by a user to having a callback fired?
Using your vaunted "I want every literal String to fire initialize" concept - when a user "requires" a file at least 10-15 strings are created in the background for a variety of reasons - is your argument that the initialize method shouldn't fire for those because the user didn't create the strings? Where does one draw the line?
The undef_method call performs a specific task which you have made an assumption as to why it would be done. Lets assume we look at a variation of the example in the docs
class Parent
def hello
puts "In parent"
end
end
class Child < Parent
end
c = Child.new
c.hello
class Child
undef_method :hello # prevent any calls to 'hello'
end
c.hello
Outputs:
In Parent
prog.rb:23: undefined method `hello' for #Child:0x401b3bb4 (NoMethodError)
In this case I am most certainly adding a method to my Child class because I'm specifically stopping a call from going up to Parent when #hello is called. Whether I use a convenience method like undef_method or write something to throw the method myself - I need to insert something into the call chain to prevent #hello from firing on the Parent class. You've made the assumption, incorrectly, that this can only be used on methods that have been added to a class previous to a undef_method call - that's simply not the case at all.
Folks again have taken a reasonable step to ensure that both speed and flexibility are met.
You are at no disadvantage in terms of understanding what is occurring either because the callbacks fire in a deterministic order for you. You know that if you have a "undefined" callback fire that there is a "added" callback that has just fired - you can easily revert any tracking that the added callback did for you if you wanted to (except if you want to know which methods actual exist on a class because undef_method certainly can add a method to a class).
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
John Higgins wrote:
[...]
Mr. Higgins, you could write a book in your non-technical tenor about this issue, the fact remains:
- callback: added any_method # false callback, "any_method" was not added by user
I'll await the response of the related developer.
Updated by wishdev (John Higgins) over 13 years ago
Lazaridis Ilias wrote:
John Higgins wrote:
[...]Mr. Higgins, you could write a book in your non-technical tenor about this issue, the fact remains:
- callback: added any_method # false callback, "any_method" was not added by user
Would you care to, even in passing, explain how I'm incorrect when it comes to the Child class example I gave? Or does clear, simple, 13 line examples showing specific cases where your hypothesis is incorrect too "non-technical a tenor" for you?
You are nothing if not consistent at least........
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
John Higgins wrote:
Lazaridis Ilias wrote:
John Higgins wrote:
[...]Mr. Higgins, you could write a book in your non-technical tenor about this issue, the fact remains:
- callback: added any_method # false callback, "any_method" was not added by user
Would you care to, even in passing, explain how I'm incorrect when it comes to the Child class example I gave?
[...]
No, I can't, because it is irrelevant (and thus I do not invest any energy to analyse). I've provided a use-case. In context of this use-case, the callback "method_added :capitalize" is false, and thus a defect. There's really no need to change context or use-cases: the defect in this use-case remains.
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
- File method_added_callback.diff added
[edit: please ignore the first patch send (32kB), the 1001 Bytes one is the correct]
Attached a simple patch, which avoids the method_added callback for
VM_METHOD_TYPE_UNDEF
OUTPUT:
undef method (buildin)
callback: undefined capitalize
define method
callback: added any_method
undef method
callback: undefined any_method
define method
callback: added any_method
remove method
callback: removed any_method
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
Updated by steveklabnik (Steve Klabnik) over 13 years ago
+1 on 'not a defect.'
If a method is added, I expect the method_added callback to be fired. undef_method adds a method. This is pretty open-and-shut.
Updated by wishdev (John Higgins) over 13 years ago
Lazaridis Ilias wrote:
John Higgins wrote:
Lazaridis Ilias wrote:
John Higgins wrote:
[...]Mr. Higgins, you could write a book in your non-technical tenor about this issue, the fact remains:
- callback: added any_method # false callback, "any_method" was not added by user
Would you care to, even in passing, explain how I'm incorrect when it comes to the Child class example I gave?
[...]No, I can't, because it is irrelevant (and thus I do not invest any energy to analyse). I've provided a use-case. In context of this use-case, the callback "method_added :capitalize" is false, and thus a defect. There's really no need to change context or use-cases: the defect in this use-case remains.
Again, what are you basing this on? I asked you what rule/concept states that internal implementations cannot fire callbacks. Why is it an error if the implementation of undef_method adds a method to the class? Where is the definition of undef_method that states anything about its implementation? In fact the docs state only the following "Prevents the current class from responding to calls to the named method." - why exactly is that at odds with a method_added callback being fired during the execution of an undef_method call?
Would you only call this a bug in a class with a "direct" definition of a method (i.e. non-inherited method). What about the user that wants to track the addition of the "blocking" method to the subclass if you wanted the method_added subclass suppressed even in a subclass without a direct definition of a method?
Updated by lazaridis.com (Lazaridis Ilias) over 13 years ago
John Higgins wrote:
[...]
no comment.
"I'll await the response of the related developer."
Updated by wishdev (John Higgins) over 13 years ago
Lazaridis Ilias wrote:
John Higgins wrote:
[...]no comment.
"I'll await the response of the related developer."
Very technical response - thanks for the rebuttal, I'm sure the "related developer" will be more than happy to implement a patch which turns off a callback that clearly and without question is valid in a multitude of cases. Your reasoned argument clearly shows the error of my ways.......
Thanks again!
Updated by antares (Michael Klishin) over 13 years ago
Lazaridis Ilias: changing behaviors of the fundamental classes because of one use case doesn't feel right. In fact, it sounds scary.
Updated by matz (Yukihiro Matsumoto) over 13 years ago
- File deleted (
method_added_callback.diff)
Updated by matz (Yukihiro Matsumoto) over 13 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r32527.
Lazaridis, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- vm_method.c (rb_add_method): should not call method_added hook
for undef operation. [Bug #5015]