Issue #20912 has been updated by jhawthorn (John Hawthorn). https://github.com/ruby/ruby/pull/12177 ---------------------------------------- Feature #20912: Move warning when redefining object_id to __id__ https://bugs.ruby-lang.org/issues/20912#change-110756 * Author: jhawthorn (John Hawthorn) * Status: Open ---------------------------------------- Currently if you create a class and redefine or remove either `object_id` or `__send__` it will issue a warning. There's no such warning on `__id__`. ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' -e:1: warning: redefining `object_id' may cause serious problems ❯ ruby -we 'class << Object.new; def __id__ = 1; end' ❯ ``` It makes sense that there's no warning on `send`, because we expect `__send__` to be the method reliably available. `__send__` is on `BasicObject`, `send` is only on `Kernel`. This seems a little inconsistent that `object_id` warns while `__id__` does not warn. `__id__` is the equivalent to `__send__` as it's also on `BasicObject`, where `object_id` is only on `Kernel`. This may be a more obvious problem in Ruby 3.4.0 as commit:cffd380390967d17899fde0a81f5151ae6ddd076 makes the warning print in more cases. I propose we change this warning to be emitted only when `__id__` is redefined and not when `object_id` is redefined: **Proposed behaviour:** ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' ❯ ruby -we 'class << Object.new; def __id__ = 1; end' -e:1: warning: redefining `__id__' may cause serious problems ❯ ``` -- https://bugs.ruby-lang.org/

Issue #20912 has been updated by jhawthorn (John Hawthorn). nobu (Nobuyoshi Nakada) wrote in #note-2:

As for `object_id`, do you want to redefine it?

I think it's best not to redefine, but a warning is probably more severe than is needed. I don't think it causes "serious problems", especially because since `object_id` doesn't exist on `BasicObject` calling code can't expect all objects to have the default implementation of `object_id`. ---------------------------------------- Feature #20912: Move warning when redefining object_id to __id__ https://bugs.ruby-lang.org/issues/20912#change-110760 * Author: jhawthorn (John Hawthorn) * Status: Open ---------------------------------------- Currently if you create a class and redefine or remove either `object_id` or `__send__` it will issue a warning. There's no such warning on `__id__`. ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' -e:1: warning: redefining `object_id' may cause serious problems ❯ ruby -we 'class << Object.new; def __id__ = 1; end' ❯ ``` It makes sense that there's no warning on `send`, because we expect `__send__` to be the method reliably available. `__send__` is on `BasicObject`, `send` is only on `Kernel`. This seems a little inconsistent that `object_id` warns while `__id__` does not warn. `__id__` is the equivalent to `__send__` as it's also on `BasicObject`, where `object_id` is only on `Kernel`. This may be a more obvious problem in Ruby 3.4.0 as commit:cffd380390967d17899fde0a81f5151ae6ddd076 makes the warning print in more cases. I propose we change this warning to be emitted only when `__id__` is redefined and not when `object_id` is redefined: **Proposed behaviour:** ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' ❯ ruby -we 'class << Object.new; def __id__ = 1; end' -e:1: warning: redefining `__id__' may cause serious problems ❯ ``` -- https://bugs.ruby-lang.org/

Issue #20912 has been updated by mame (Yusuke Endoh). I think it's a good idea to warn against redefining `__id__`, but I think we should keep warning against redefining `object_id` as well. The reason is that it is relatively well known that "we should use `__send__` instead of `send` for unknown objects", but I think it is less well known that "we should use `__id__` instead of `object_id`". In fact, `object_id` is called far more often than `__id__`. ``` $ gem-codesearch \\.object_id | wc -l 24048 $ gem-codesearch \\.__id__ | wc -l 3763 ``` I don't know how many of these call object_id for instances of user-defined classes, but such gems may not work properly if `object_id` is redefined. If we really want to allow the redefinition of `object_id`, we will need to create a consensus that a call to `object_id` for unknown objects is dangerous, and get people to fix gems so that `__id__` is used instead of `object_id` for unknown objects. However, if you are currently not in that much trouble, I guess it would be better to maintain the current status. --- Incidentally, the names `__id__` and `object_id` have an interesting history. * There used to be only `Object#id`. * `Object#__id__` was introduced to avoid the redefinition of the method name `id`. * `Object#object_id` was also introduced. * `Object#id` was completely removed to allow application authors to use tje name `id` freely. * And `__id__` and `object_id` were left. I asked matz why the new name `object_id` was introduced when `__id__` was already there. He answered, "I didn't want to have to always use `__id__` because it would be ugly". https://twitter.com/yukihiro_matz/status/1861726864332742995 Indeed, it's not very cool to call `__id__` on an object that you know you don't have to worry about redefining. ---------------------------------------- Feature #20912: Move warning when redefining object_id to __id__ https://bugs.ruby-lang.org/issues/20912#change-110765 * Author: jhawthorn (John Hawthorn) * Status: Open ---------------------------------------- Currently if you create a class and redefine or remove either `object_id` or `__send__` it will issue a warning. There's no such warning on `__id__`. ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' -e:1: warning: redefining `object_id' may cause serious problems ❯ ruby -we 'class << Object.new; def __id__ = 1; end' ❯ ``` It makes sense that there's no warning on `send`, because we expect `__send__` to be the method reliably available. `__send__` is on `BasicObject`, `send` is only on `Kernel`. This seems a little inconsistent that `object_id` warns while `__id__` does not warn. `__id__` is the equivalent to `__send__` as it's also on `BasicObject`, where `object_id` is only on `Kernel`. This may be a more obvious problem in Ruby 3.4.0 as commit:cffd380390967d17899fde0a81f5151ae6ddd076 makes the warning print in more cases. I propose we change this warning to be emitted only when `__id__` is redefined and not when `object_id` is redefined: **Proposed behaviour:** ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' ❯ ruby -we 'class << Object.new; def __id__ = 1; end' -e:1: warning: redefining `__id__' may cause serious problems ❯ ``` -- https://bugs.ruby-lang.org/

Issue #20912 has been updated by Eregon (Benoit Daloze). mame (Yusuke Endoh) wrote in #note-5:

I think it's a good idea to warn against redefining `__id__`, but I think we should keep warning against redefining `object_id` as well.

The reason is that it is relatively well known that "we should use `__send__` instead of `send` for unknown objects", but I think it is less well known that "we should use `__id__` instead of `object_id`". In fact, `object_id` is called far more often than `__id__`.

I agree and I think this a well thought out argument. In fact I used object_id plenty of times but almost never `__id__`.

He answered, "I didn't want to have to always use `__id__` because it would be ugly".

Same feeling here, I always prefer to use send/object_id when it's safe, because I think it looks much better/is more readable. Maybe it would even make sense to warn for `send` to discourage overriding it, but out of scope and probably not worth it (considering e.g. Socket#send in stdlib).

Is object_id really called that much?

It seems used a lot, yes: https://github.com/search?q=%2F%5C.object_id%5Cb%2F+language%3ARuby+&type=code&p=1 ---------------------------------------- Feature #20912: Move warning when redefining object_id to __id__ https://bugs.ruby-lang.org/issues/20912#change-110777 * Author: jhawthorn (John Hawthorn) * Status: Open ---------------------------------------- Currently if you create a class and redefine or remove either `object_id` or `__send__` it will issue a warning. There's no such warning on `__id__`. ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' -e:1: warning: redefining `object_id' may cause serious problems ❯ ruby -we 'class << Object.new; def __id__ = 1; end' ❯ ``` It makes sense that there's no warning on `send`, because we expect `__send__` to be the method reliably available. `__send__` is on `BasicObject`, `send` is only on `Kernel`. This seems a little inconsistent that `object_id` warns while `__id__` does not warn. `__id__` is the equivalent to `__send__` as it's also on `BasicObject`, where `object_id` is only on `Kernel`. This may be a more obvious problem in Ruby 3.4.0 as commit:cffd380390967d17899fde0a81f5151ae6ddd076 makes the warning print in more cases. I propose we change this warning to be emitted only when `__id__` is redefined and not when `object_id` is redefined: **Proposed behaviour:** ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' ❯ ruby -we 'class << Object.new; def __id__ = 1; end' -e:1: warning: redefining `__id__' may cause serious problems ❯ ``` -- https://bugs.ruby-lang.org/

Issue #20912 has been updated by matz (Yukihiro Matsumoto). I am strongly discourage redefining `object_id`, so we should keep the warning. I am not against warning `__id__` redefinition. Matz. ---------------------------------------- Feature #20912: Move warning when redefining object_id to __id__ https://bugs.ruby-lang.org/issues/20912#change-110787 * Author: jhawthorn (John Hawthorn) * Status: Open ---------------------------------------- Currently if you create a class and redefine or remove either `object_id` or `__send__` it will issue a warning. There's no such warning on `__id__`. ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' -e:1: warning: redefining `object_id' may cause serious problems ❯ ruby -we 'class << Object.new; def __id__ = 1; end' ❯ ``` It makes sense that there's no warning on `send`, because we expect `__send__` to be the method reliably available. `__send__` is on `BasicObject`, `send` is only on `Kernel`. This seems a little inconsistent that `object_id` warns while `__id__` does not warn. `__id__` is the equivalent to `__send__` as it's also on `BasicObject`, where `object_id` is only on `Kernel`. This may be a more obvious problem in Ruby 3.4.0 as commit:cffd380390967d17899fde0a81f5151ae6ddd076 makes the warning print in more cases. I propose we change this warning to be emitted only when `__id__` is redefined and not when `object_id` is redefined: **Proposed behaviour:** ``` ❯ ruby -we 'class << Object.new; def object_id = 1; end' ❯ ruby -we 'class << Object.new; def __id__ = 1; end' -e:1: warning: redefining `__id__' may cause serious problems ❯ ``` -- https://bugs.ruby-lang.org/