Feature #1586

Including a module already present in ancestors should not be ignored

Added by Jeremy Kemper almost 5 years ago. Updated over 1 year ago.

[ruby-core:23740]
Status:Assigned
Priority:Normal
Assignee:Yukihiro Matsumoto
Category:core
Target version:Next Major

Description

=begin
The scenario:
* I include Foo in Numeric to provide #bar
* Some other library includes a module in Float to provide #bar
* So I include Foo in Float to use my #bar
* But including Foo in Float is ignored since it's already in the ancestor chain

I think it should be added to the ancestor chain, even if it's already present, since I may want to override some other method earlier in the ancestor chain.

# Including a module already included in a superclass is ignored

module Foo; end
=> nil
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]
class Float; include Foo; end
=> Float
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]

# Reversing the order of inclusion works as expected

module Foo; end
=> nil
class Float; include Foo; end
=> Float
Float.ancestors
=> [Float, Foo, Precision, Numeric, Comparable, Object, Kernel]
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Foo, Precision, Numeric, Foo, Comparable, Object, Kernel]

# And so does including a dupe of the existing module in the subclass

module Foo; end
=> nil
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]
class Float; include Foo.dup; end
=> Float
Float.ancestors
=> [Float, #Module:0x19bcd40, Precision, Numeric, Foo, Comparable, Object, Kernel]
=end

inclusion.pdf (56.4 KB) Clay Trump, 07/01/2012 08:53 AM


Related issues

Related to ruby-trunk - Bug #5236: Including a module in a superclass after it has been incl... Closed 08/27/2011
Related to ruby-trunk - Bug #3351: stack overflow on super Open 05/27/2010
Duplicated by ruby-trunk - Bug #8066: Inconsistency in ancestors chain Open 03/10/2013

History

#1 Updated by Nobuyoshi Nakada almost 5 years ago

  • Category set to core
  • Assignee set to Yukihiro Matsumoto
  • Target version set to Next Major

=begin

=end

#2 Updated by Rick DeNatale almost 5 years ago

=begin
Actually, for a while, back in 2006, Ruby 1.9 (in its experimental form) used to do just what this ticket asks for:

http://talklikeaduck.denhaven2.com/2006/10/09/a-subtle-change-to-mixin-semantics-in-ruby-1-9

However, that change got reverted.

I asked Matz why at RubyConf 2007, and documented our conversion

http://talklikeaduck.denhaven2.com/2007/11/03/a-chat-with-matz-classs-variable-reversion-and-a-mystery-explained

The problem is that MRI, and I guess YARV doesn't keep track of where in the chain of classes, and module proxies it found the currently executing method, so super is implemented by doing a method search starting with the klass of self, and proceding until the method is found a SECOND time. With this implementation it's easier to turn module re-inclusion into a nop than to deal with the consequences.

=end

#3 Updated by Jeremy Kemper almost 5 years ago

=begin
Fascinating. Thanks for the history behind this, Rick.

Despite the implementation difficulties, I'd like to see this choice revisited for a future Ruby. I consider it a bug.
=end

#4 Updated by Shyouhei Urabe over 3 years ago

  • Status changed from Open to Assigned

=begin

=end

#5 Updated by Yui NARUSE over 2 years ago

  • Project changed from ruby-trunk to CommonRuby
  • Category deleted (core)
  • Target version deleted (Next Major)

#6 Updated by Yui NARUSE over 2 years ago

  • Project changed from CommonRuby to ruby-trunk

#7 Updated by Yusuke Endoh about 2 years ago

  • Status changed from Assigned to Rejected

I'm rejecting this feature ticket because no progress has been
made for a long time. See .

This is indeed a hard issue.
There are two approaches to make it consistent:

A) prohibit multiple appearances of one module in ancestors
B) permit multiple appearances

I guess matz prefers A to B, so B will not be admissible. (just
my guess, though) The current behavior aims A, but is indeed
incomplete.

In casual use case, however, module inclusion is used statically,
i.e., used when a class is first defined.
Thus, no actual problem is caused in practical case, I think.

In addition, A will be very hard to implement completely. Even
if it is possible, I'm afraid that the behavior will be bug-prone
(for both user code and ruby impl), because it means that an
already-included module may be later removed from ancestors,
unexpectedly.

Thus, I think that we can not fix this elegantly and will not
change the current behavior. But we might have to do something
if there is any concrete scenario in that the current behavior
causes a trouble. Let us know such a scenario if you have.

Yusuke Endoh mame@tsg.ne.jp

#8 Updated by Koichi Sasada about 2 years ago

  • Category set to core
  • Status changed from Rejected to Open
  • Target version set to 2.0.0

I want to change this behavior as permitting multiple modules in an ancestors list (option B).

Can I do it? (If I can, I'll implement it after this Apr).

#9 Updated by Marc-Andre Lafortune about 2 years ago

Koichi Sasada wrote:

I want to change this behavior as permitting multiple modules in an ancestors list (option B).

Can I do it? (If I can, I'll implement it after this Apr).

That would be great.

I'm assuming it be allowed to include a module multiple times in a row too?

module M
def to_s
"" + super + ""
end
end

3.times{ Fixnum.prepend(M) }
42.to_s # => "42"

#10 Updated by Shyouhei Urabe about 2 years ago

  • Status changed from Open to Assigned

#11 Updated by Marc-Andre Lafortune almost 2 years ago

Has this feature been accepted by Matz? Or else, is anyone producing a one minute slide-show?

#12 Updated by Clay Trump almost 2 years ago

I guess I'll do it. Hopefully this slide can be useful.

On Sat, Jun 23, 2012 at 2:20 PM, marcandre (Marc-Andre Lafortune) <
ruby-core@marc-andre.ca> wrote:

Issue #1586 has been updated by marcandre (Marc-Andre Lafortune).

Has this feature been accepted by Matz? Or else, is anyone producing a

one minute slide-show?

Feature #1586: Including a module already present in ancestors should not
be ignored
https://bugs.ruby-lang.org/issues/1586#change-27383

Author: bitsweat (Jeremy Kemper)
Status: Assigned
Priority: Normal
Assignee: matz (Yukihiro Matsumoto)
Category: core
Target version: 2.0.0

=begin
The scenario:
* I include Foo in Numeric to provide #bar
* Some other library includes a module in Float to provide #bar
* So I include Foo in Float to use my #bar
* But including Foo in Float is ignored since it's already in the
ancestor chain

I think it should be added to the ancestor chain, even if it's already
present, since I may want to override some other method earlier in the
ancestor chain.

# Including a module already included in a superclass is ignored

module Foo; end
=> nil
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]
class Float; include Foo; end
=> Float
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]

# Reversing the order of inclusion works as expected

module Foo; end
=> nil
class Float; include Foo; end
=> Float
Float.ancestors
=> [Float, Foo, Precision, Numeric, Comparable, Object, Kernel]
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Foo, Precision, Numeric, Foo, Comparable, Object, Kernel]

# And so does including a dupe of the existing module in the subclass

module Foo; end
=> nil
class Numeric; include Foo; end
=> Numeric
Float.ancestors
=> [Float, Precision, Numeric, Foo, Comparable, Object, Kernel]
class Float; include Foo.dup; end
=> Float
Float.ancestors
=> [Float, #Module:0x19bcd40, Precision, Numeric, Foo, Comparable,
Object, Kernel]
=end

http://bugs.ruby-lang.org/

--

#13 Updated by Yusuke Endoh almost 2 years ago

Hi Clay and MarcAndre,

I received your slide. Thanks!

Yusuke Endoh mame@tsg.ne.jp

#14 Updated by Yusuke Endoh over 1 year ago

  • Assignee changed from Yukihiro Matsumoto to Koichi Sasada

Jeremy Kemper and Clay Trump,

This proposal was discussed as two separate ones:

  1. allow multiple inclusion
  2. propagate when a module includes a module

Both have been (basically) accepted. Congrats!

For part (1), matz said that he had tried to implemented this
feature for 1.9.0, but had given up due to a bug of YARV (#3351).
Ko1 will challenge to fix the bug.

For part (2), the behavior was designed because matz had no idea
to implement it effectively. However, after the discussion, we
concluded it was possible to implement the feature effectively.
Then, matz said he was happy to change the current behavior as
a "bug". Ko1 will challange to implement it.

Note that these decisions may be cancelled if ko1 finds any
(practical or technical) significant problem.

Yusuke Endoh mame@tsg.ne.jp

#15 Updated by Koichi Sasada over 1 year ago

  • Priority changed from Normal to High

Nobu, could you help to implement (1)?

I'm not sure (2) can get 2.0 release. Sorry.

#16 Updated by Clay Trump over 1 year ago

I'm looking forward to consistent module inclusion :-)

I should have talked about Marc-andre's example with multiple inclusion at
the same level (same class/module/object), coz I don't think it should be
done this way. The example was:

 module M
   def to_s
     "*" + super + "*"
   end
 end

 3.times{ Fixnum.prepend(M) }
 42.to_s # => "*42*" would be best, not "***42***"

My reasoning is that otehrwise it makes it hard to reload a Ruby class
definition. When debugging, it is frequent that one modifies a method
definition, reloads the whole file and tests away. It would not be
practical if any +include+ or +prepend+ in the class definition induced a
change in the ancestry.

If someone really wants multiple inclusion at the same level, it can be
achieved by dupping the module.

So I believe that a module should only be included once per
class/module/object. Same thing for prepending. Prepending and including
should be independent though.

 3.times{ Fixnum.prepend(M) }
 2.times{ Fixnum.include(M) }
 Numeric.prepend(M)
 Fixnum.ancestors # => [M, Fixnum, M, M, Numeric, ...], not [M, M, M,

Fixnum, M, M, M, Numeric, ...]

Is that the plan? Was this brought up when discussing with Matz about this?

#17 Updated by Yusuke Endoh over 1 year ago

ko1, what's the status?

Yusuke Endoh mame@tsg.ne.jp

#18 Updated by Koichi Sasada over 1 year ago

  • Assignee changed from Koichi Sasada to Nobuyoshi Nakada

nobu, could you check it?

#19 Updated by Nobuyoshi Nakada over 1 year ago

  • Priority changed from High to Normal
  • Target version changed from 2.0.0 to next minor

This would cause compatibility issue, in some cases, when a module is
included twice but it expects it never get called twice or more by
super. Or, if a method of the module does not call super, the super
calls stops unexpectedly.

One idea is to introduce a method of Module which tells a module is
multi-time includable.

#20 Updated by Nobuyoshi Nakada over 1 year ago

  • Assignee changed from Nobuyoshi Nakada to Yukihiro Matsumoto

#21 Updated by Marc-Andre Lafortune over 1 year ago

As Clay said, I think the example I gave previously is not the way to go.

The important aspect is the ability to include a module at different levels in the hierarchy.

If C < B < A, we should be able to include a module M for each of A, B and C, but only once for each of them.

There should be no incompatibility this way.

#22 Updated by Yukihiro Matsumoto over 1 year ago

  • Target version changed from next minor to Next Major

When I made this change in early 1.9, it caused huge incompatibility.
I had to fix many bundled programs. So it shouldn't be a minor change.

Matz.

#23 Updated by Marc-Andre Lafortune over 1 year ago

matz (Yukihiro Matsumoto) wrote:

When I made this change in early 1.9, it caused huge incompatibility.
I had to fix many bundled programs. So it shouldn't be a minor change.

When you did this, was it possible to include a module more than once for the same class, or simply possible once for any class/module, even when it present somewhere else in the ancestor chain? (See http://bugs.ruby-lang.org/issues/1586#note-16 for an example)

I believe that allowing inclusion of a module once per module is the most useful and would not cause incompatibility.

#24 Updated by Boris Stitnicky over 1 year ago

I join my voice with Marc-Andre. (More precisely, a module in the "is kind of?" sense should be included or not included only once, globally. But tweaking the method lookup sequence is imaginable.)

#25 Updated by Yukihiro Matsumoto over 1 year ago

My change at the beginning of 1.9 was include module once for a class/module, as you described, and still it caused incompatibility. I had to change hundreds of lines in the standard libraries. So I think this change is good but we need to make it in proper timing.

Matz.

Also available in: Atom PDF