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. fix Module inclusion order (fixes #1938)

    @MSNexploder
    MSNexploder committed Jun 3, 2017
    Copy the full SHA
    a7aef67 View commit details

  2. enable Module include order test

    @MSNexploder
    MSNexploder committed Jun 3, 2017
    Copy the full SHA
    fbf207c View commit details

  3. remove unused internal method

    @MSNexploder
    MSNexploder committed Jun 3, 2017
    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
    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
View file
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
View file
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
2 changes: 1 addition & 1 deletion test/test_include_order.rb → test/jruby/test_include_order.rb
View file
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