jruby · Jun 5, 2015 · Jun 5, 2015
diff --git a/spec/truffle/tags/core/module/include_tags.txt b/spec/truffle/tags/core/module/include_tags.txt
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ModuleNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ModuleNodes.java
@@ -351,13 +351,13 @@ public AppendFeaturesNode(RubyContext context, SourceSection sourceSection) {
         }
 
         @Specialization
-        public RubyBasicObject appendFeatures(RubyModule module, RubyModule other) {
-            if (module instanceof RubyClass) {
+        public RubyBasicObject appendFeatures(RubyModule features, RubyModule target) {
+            if (features instanceof RubyClass) {
                 CompilerDirectives.transferToInterpreter();
                 throw new RaiseException(getContext().getCoreLibrary().typeError("append_features must be called only on modules", this));
             }
-            module.appendFeatures(this, other);
-            taintResultNode.maybeTaint(module, other);
+            target.include(this, features);
+            taintResultNode.maybeTaint(features, target);
             return nil();
         }
     }

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/ModuleChain.java b/truffle/src/main/java/org/jruby/truffle/runtime/ModuleChain.java
@@ -19,4 +19,6 @@ public interface ModuleChain {
     ModuleChain getParentModule();
 
     RubyModule getActualModule();
+
+    void insertAfter(RubyModule module);
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/core/RubyModule.java b/truffle/src/main/java/org/jruby/truffle/runtime/core/RubyModule.java
@@ -36,7 +36,7 @@ public class RubyModule extends RubyBasicObject implements ModuleChain {
      */
     private static class IncludedModule implements ModuleChain {
         private final RubyModule includedModule;
-        private final ModuleChain parentModule;
+        @CompilationFinal private ModuleChain parentModule;
 
         public IncludedModule(RubyModule includedModule, ModuleChain parentModule) {
             this.includedModule = includedModule;
@@ -57,6 +57,11 @@ public RubyModule getActualModule() {
         public String toString() {
             return super.toString() + "(" + includedModule + ")";
         }
+
+        @Override
+        public void insertAfter(RubyModule module) {
+            parentModule = new IncludedModule(module, parentModule);
+        }
     }
 
     public static void debugModuleChain(RubyModule module) {
@@ -171,22 +176,65 @@ private void checkFrozen(Node currentNode) {
         }
     }
 
+    @Override
+    public void insertAfter(RubyModule module) {
+        parentModule = new IncludedModule(module, parentModule);
+    }
+
     @TruffleBoundary
     public void include(Node currentNode, RubyModule module) {
         checkFrozen(currentNode);
 
-        // We need to traverse the module chain in reverse order
-        Stack<RubyModule> moduleAncestors = new Stack<>();
+        // If the module we want to include already includes us, it is cyclic
+        if (ModuleOperations.includesModule(module, this)) {
+            throw new RaiseException(getContext().getCoreLibrary().argumentError("cyclic include detected", currentNode));
+        }
+
+        // We need to include the module ancestors in reverse order for a given inclusionPoint
+        ModuleChain inclusionPoint = this;
+        Stack<RubyModule> modulesToInclude = new Stack<>();
         for (RubyModule ancestor : module.ancestors()) {
-            moduleAncestors.push(ancestor);
+            if (ModuleOperations.includesModule(this, ancestor)) {
+                if (isIncludedModuleBeforeSuperClass(ancestor)) {
+                    // Include the modules at the appropriate inclusionPoint
+                    performIncludes(inclusionPoint, modulesToInclude);
+                    assert modulesToInclude.isEmpty();
+
+                    // We need to include the others after that module
+                    inclusionPoint = parentModule;
+                    while (inclusionPoint.getActualModule() != ancestor) {
+                        inclusionPoint = inclusionPoint.getParentModule();
+                    }
+                } else {
+                    // Just ignore this module, as it is included above the superclass
+                }
+            } else {
+                modulesToInclude.push(ancestor);
+            }
         }
 
+        performIncludes(inclusionPoint, modulesToInclude);
+
+        newVersion();
+    }
+
+    private void performIncludes(ModuleChain inclusionPoint, Stack<RubyModule> moduleAncestors) {
         while (!moduleAncestors.isEmpty()) {
             RubyModule mod = moduleAncestors.pop();
-            parentModule = new IncludedModule(mod, parentModule);
+            inclusionPoint.insertAfter(mod);
             mod.addDependent(this);
         }
-        newVersion();
+    }
+
+    private boolean isIncludedModuleBeforeSuperClass(RubyModule module) {
+        boolean isDirectlyIncluded = false;
+        for (RubyModule includedModule : includedModules()) {
+            if (includedModule == module) {
+                isDirectlyIncluded = true;
+                break;
+            }
+        }
+        return isDirectlyIncluded;
     }
 
     /**
@@ -347,14 +395,6 @@ public void changeConstantVisibility(Node currentNode, String name, boolean isPr
         }
     }
 
-    @TruffleBoundary
-    public void appendFeatures(Node currentNode, RubyModule other) {
-        if (ModuleOperations.includesModule(this, other)) {
-            throw new RaiseException(getContext().getCoreLibrary().argumentError("cyclic include detected", currentNode));
-        }
-        other.include(currentNode, this);
-    }
-
     public RubyContext getContext() {
         return context;
     }