Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 6e79fafe2556
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: d47687ac2de0
Choose a head ref
  • 4 commits
  • 3 files changed
  • 2 contributors

Commits on Jun 3, 2017

  1. Copy the full SHA
    a7aef67 View commit details
  2. Copy the full SHA
    fbf207c View commit details
  3. Copy the full SHA
    5b036d9 View commit details

Commits on Jun 6, 2017

  1. Merge pull request #4639 from MSNexploder/module_order

    Fix Module inclusion order
    headius authored Jun 6, 2017
    Copy the full SHA
    d47687a View commit details
Showing with 22 additions and 27 deletions.
  1. +20 −26 core/src/main/java/org/jruby/RubyModule.java
  2. +1 −0 test/jruby.index
  3. +1 −1 test/{ → jruby}/test_include_order.rb
46 changes: 20 additions & 26 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -2936,14 +2936,17 @@ private void doIncludeModule(RubyModule baseModule) {
// nextClass.isIncluded() && nextClass.getNonIncludedClass() == nextModule.getNonIncludedClass();
// scan class hierarchy for module
for (RubyClass nextClass = methodLocation.getSuperClass(); nextClass != null; nextClass = nextClass.getSuperClass()) {
if (doesTheClassWrapTheModule(nextClass, nextModule)) {
// next in hierarchy is an included version of the module we're attempting,
// so we skip including it
if (nextClass.isIncluded()) {
// does the class equal the module
if (nextClass.getDelegate() == nextModule.getDelegate()) {
// next in hierarchy is an included version of the module we're attempting,
// so we skip including it

// if we haven't encountered a real superclass, use the found module as the new inclusion point
if (!superclassSeen) currentInclusionPoint = nextClass;
// if we haven't encountered a real superclass, use the found module as the new inclusion point
if (!superclassSeen) currentInclusionPoint = nextClass;

continue ModuleLoop;
continue ModuleLoop;
}
} else {
superclassSeen = true;
}
@@ -2992,36 +2995,27 @@ private void doPrependModule(RubyModule baseModule) {
boolean superclassSeen = false;

// scan class hierarchy for module
for (RubyClass nextClass = this.getSuperClass(); nextClass != null; nextClass = nextClass.getSuperClass()) {
if (doesTheClassWrapTheModule(nextClass, nextModule)) {
// next in hierarchy is an included version of the module we're attempting,
// so we skip including it
for (RubyClass nextClass = methodLocation.getSuperClass(); nextClass != null; nextClass = nextClass.getSuperClass()) {
if (nextClass.isIncluded()) {
// does the class equal the module
if (nextClass.getDelegate() == nextModule.getDelegate()) {
// next in hierarchy is an included version of the module we're attempting,
// so we skip including it

// if we haven't encountered a real superclass, use the found module as the new inclusion point
if (!superclassSeen) currentInclusionPoint = nextClass;
// if we haven't encountered a real superclass, use the found module as the new inclusion point
if (!superclassSeen) currentInclusionPoint = nextClass;

continue ModuleLoop;
continue ModuleLoop;
}
} else {
superclassSeen = true;
}
}

currentInclusionPoint = proceedWithPrepend(currentInclusionPoint, nextModule);
currentInclusionPoint = proceedWithPrepend(currentInclusionPoint, nextModule.getDelegate());
}
}

/**
* Is the given class a wrapper for the specified module?
*
* @param theClass The class to inspect
* @param theModule The module we're looking for
* @return true if the class is a wrapper for the module, false otherwise
*/
private boolean doesTheClassWrapTheModule(RubyClass theClass, RubyModule theModule) {
return theClass.isIncluded() &&
theClass.getDelegate() == theModule.getDelegate();
}

/**
* Gather all modules that would be included by including the given module.
* The resulting list contains the given module and its (zero or more)
1 change: 1 addition & 0 deletions test/jruby.index
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ jruby/test_frame_self
jruby/test_hash
jruby/test_higher_javasupport
jruby/test_ifaddr
jruby/test_include_order
jruby/test_included_in_object_space
jruby/test_integer_overflows
jruby/test_ivar_table_integrity
Original file line number Diff line number Diff line change
@@ -44,4 +44,4 @@ class Y
def test_include_order
assert_equal Y.new.foo, [:Z, :A, :Q]
end
end
end