Bug #5015: method_added" is called in addition to "method_undefined

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 1.9.2

[lazaridis.com (Lazaridis Ilias)]

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

[judofyr (Magnus Holm)]

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.

[lazaridis.com (Lazaridis Ilias)]

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

[wishdev (John Higgins)]

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).

[lazaridis.com (Lazaridis Ilias)]

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.

[wishdev (John Higgins)]

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........

[lazaridis.com (Lazaridis Ilias)]

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.

[lazaridis.com (Lazaridis Ilias)]

• 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

[lazaridis.com (Lazaridis Ilias)]

• File method_added_callback.diff method_added_callback.diff added

[steveklabnik (Steve Klabnik)]

+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.

[wishdev (John Higgins)]

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?

[lazaridis.com (Lazaridis Ilias)]

John Higgins wrote:
[...]

no comment.

"I'll await the response of the related developer."

[wishdev (John Higgins)]

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!

[antares (Michael Klishin)]

Lazaridis Ilias: changing behaviors of the fundamental classes because of one use case doesn't feel right. In fact, it sounds scary.

[matz (Yukihiro Matsumoto)]

• File deleted (method_added_callback.diff)

[matz (Yukihiro Matsumoto)]

• 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]