Project

General

Profile

Actions

Bug #7844

closed

include/prepend satisfiable module dependencies are not satisfied

Added by mame (Yusuke Endoh) over 11 years ago. Updated about 3 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 2.0.0dev (2013-02-13 trunk 39225) [x86_64-linux]
Backport:
[ruby-core:52208]
Tags:

Description

Hello,

module P
  def m; puts "P"; super; end
end
module Q
  def m; puts "Q"; super; end
  include P
end
module R
  def m; puts "R"; super; end
  prepend Q
end
module S
  def m; puts "S"; super; end
  include R
end
class A
  def m; puts "A"; super; end
  prepend P
  include S
end
A.new.m #=> P, R, A, S, Q

This code has five module dependencies, but only two are satisfied.

  • Q includes P, which is not satisfied: P#m precedes Q#m.
  • R prepends Q, which is not satisfied: R#m precedes Q#m.
  • S includes R, which is not satisfied: R#m precedes S#m.
  • A prepends P, which is satisfied: P#m precedes A#m.
  • A includes S, which is satisfied: A#m precedes S#m.

Note that all the dependencies can be satisfied at all:

A.new.m #=> Q, P, A, S, R

--
Yusuke Endoh


Files

include-after-origin-7844.patch (2.61 KB) include-after-origin-7844.patch jeremyevans0 (Jeremy Evans), 08/08/2019 05:05 PM

Related issues 3 (0 open3 closed)

Related to Ruby master - Bug #14704: Module#ancestors looks wrong when a module is both included and prepended in the same class.ClosedActions
Related to Ruby master - Bug #17423: `Prepend` should prepend a module before the classClosedjeremyevans0 (Jeremy Evans)Actions
Related to Ruby master - Bug #18064: Backport fix for bug 7844 (include/prepend satisfiable module dependencies are not satisfied)ClosedActions

Updated by matz (Yukihiro Matsumoto) over 11 years ago

I believe the behavior is undefined (or implementation defined) when module dependency has contradiction.
And preferably error should be raised when contradiction detected.

Matz.

Updated by mame (Yusuke Endoh) over 11 years ago

matz (Yukihiro Matsumoto) wrote:

I believe the behavior is undefined (or implementation defined) when module dependency has contradiction.
And preferably error should be raised when contradiction detected.

Thank you, I agree with the policy.

However, is there any 'contradiction' in this case? No pair of these dependencies conflicts. There are no cycles.
Actually, they are satisfiable. A solution can be found by using the topological sorting.

--
Yusuke Endoh

Updated by marcandre (Marc-Andre Lafortune) over 11 years ago

As I stated before (#1586), I feel that the solution is easy:

A.ancestors = [*A.prepended_modules.flat_map(&:ancestors), A, *A.included_modules.flat_map(&:ancestors), *A.superclass.ancestors]

In the given example, this would be:

  P, A, S, Q, P, R, Object, Kernel, BasicObject

It makes absolutely no sense to me that R could come before A and believe it is clearly a major problem. R is never prepended, nor included in a prepended module!

Matz: how would you explain that R can be called before A in that example?

Updated by nobu (Nobuyoshi Nakada) over 10 years ago

  • Description updated (diff)
Actions #5

Updated by naruse (Yui NARUSE) almost 7 years ago

  • Target version deleted (2.6)

Updated by jeremyevans0 (Jeremy Evans) about 5 years ago

This is still a bug in the master branch. I think the main problem in this example is that when calling Module#include, you can get a module inserted before the receiver in the lookup chain instead of behind the receiver.

mame correctly pointed out that using a topological sort you can satisfy all dependencies in the example. However, you can only do so by inserting a module before the receiver in the lookup chain, which I don't think should be allowed.

In the example, at the time A.include S is called:

A.ancestors # => [P, A, Object, Kernel, BasicObject]
S.ancestors # => [S, Q, P, R]

In my opinion, Module#include should have these properties:

(1) It should not add a module to the receiver's lookup chain if it already exists anywhere in the receiver's lookup chain
(2) It should not move the position of any module in the receiver's lookup chain, only add modules to the receiver's lookup chain
(3) It should not insert any modules before the receiver in the receiver's lookup chain
(4) It should insert all modules as a group, not some modules in the receiver's lookup chain at a different place than other modules

Applied to this example, assuming the above properties, after A.include S, we should have the following

A.ancestors # => [P, A, S, Q, R, Object, Kernel, BasicObject]

In this example, P in the included module lookup chain is ignored as it is already in the lookup chain for A. The remaining modules in the included module lookup chain are inserted as a group after A.

Attached is a patch that implements the above behavior, implementing property (3). The only change is that before inserting a module into the lookup chain, we check to make sure we are inserting it after the origin class. make check passes with this patch. I'm not sure this implements property (4). Still, I believe this is an improvement and should be committed.

Actions #7

Updated by jeremyevans0 (Jeremy Evans) almost 5 years ago

  • Related to Bug #14704: Module#ancestors looks wrong when a module is both included and prepended in the same class. added

Updated by jeremyevans0 (Jeremy Evans) about 4 years ago

@matz (Yukihiro Matsumoto) considered this during the September 2019 and December 2019 developers meetings, but has not yet made a decision on it.

Updated by matz (Yukihiro Matsumoto) almost 4 years ago

I'd like to apply the patch to see if there's any compatibility issue.

Matz.

Actions #10

Updated by jeremyevans (Jeremy Evans) almost 4 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|08686e71d5c48325ee1bd41c8d7ebcd6c37fa496.


Do not allow Module#include to insert modules before the origin in the lookup chain

Module#include should only be able to insert modules after the origin,
otherwise it ends up working like Module#prepend.

This fixes the case where one of the modules in the included module
chain is included in a module that is already prepended to the receiver.

Fixes [Bug #7844]

Actions #11

Updated by matz (Yukihiro Matsumoto) almost 4 years ago

  • Related to Bug #17423: `Prepend` should prepend a module before the class added

Updated by jeremyevans0 (Jeremy Evans) about 3 years ago

  • Backport set to 2.7: REQUIRED

PickachuEXE on GitHub requested this be backported to 2.7: https://github.com/ruby/ruby/pull/4711 . This does affect backwards compatibility, so I'm not sure it is something that should be backported. The 2.7 branch maintainer (@usa (Usaku NAKAMURA)) will have to make that determination.

Updated by PikachuEXE (Pikachu EXE) about 3 years ago

Thanks @jeremyevans0 (Jeremy Evans) for informing the backport request before I create the backport ticket

A backport ticket is created:
https://bugs.ruby-lang.org/issues/18064

I have provided the reason for requesting this backport to 2.7 in that ticket

Actions #14

Updated by Eregon (Benoit Daloze) about 3 years ago

  • Related to Bug #18064: Backport fix for bug 7844 (include/prepend satisfiable module dependencies are not satisfied) added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0