[GR-24384] Refinements and super (#2039).

PullRequest: truffleruby/1730

Author: eregon

CHANGELOG.md

@@ -58,10 +58,11 @@ Compatibility:
 * Implemented `rb_enc_isalnum` and `rb_enc_isspace`.
 * `RUBY_REVISION` is now the full commit hash used to build TruffleRuby, similar to MRI 2.7+.
 * Implemented `rb_enc_mbc_to_codepoint`.
-* Change the lookup methods to achieve Refinements specification (#2033, @ssnickolay)
+* Changed the lookup methods to achieve Refinements specification (#2033, @ssnickolay)
 * Implemented `Digest::Instance#new` (#2040).
 * Implemented `ONIGENC_MBC_CASE_FOLD`.
 * Fixed `Thread#raise` to call the exception class' constructor with no arguments when given no message (#2045).
+* Fixed `refine + super` compatibility (#2039, @ssnickolay)
 
 Performance:
 

doc/contributor/refinements.md

@@ -88,12 +88,15 @@ The lookup order:
 R1 -> A -> B -> R2 -> D -> E -> C -> ...
 ```
 
-The `super` lookup works in much the same way, except `C` ancestors have a higher priority than other active refinements.
+## Super Dispatch
 
-The lookup order for `super`:
-```ruby
-R1 -> A -> B -> C -> ... -> R2 -> D -> E -> C -> ...
-```
+
+The `super` lookup [works in two modes](https://bugs.ruby-lang.org/issues/16977):
+
+1. If `super` is called from a method is directly in R, then we should search in `C` ancestors and ignore other active refinements.
+2. If `super` is called from a method placed in a module which included to R, then we should search over all active refinements (as we do for a regular lookup).
+
+Additionally, `super` has access to the caller active refinements, so we use `InternalMethod#activeRefinements` to keep and re-use necessary refinements.
 
 ## References
 

spec/ruby/core/module/fixtures/refine.rb

@@ -3,6 +3,10 @@ class ClassWithFoo
     def foo; "foo" end
   end
 
+  class ClassWithSuperFoo
+    def foo; [:C] end
+  end
+
   module PrependedModule
     def foo; "foo from prepended module"; end
   end
@@ -11,7 +15,11 @@ module IncludedModule
     def foo; "foo from included module"; end
   end
 
-  def self.build_refined_class
-    Class.new(ClassWithFoo)
+  def self.build_refined_class(for_super: false)
+    if for_super
+      Class.new(ClassWithSuperFoo)
+    else
+      Class.new(ClassWithFoo)
+    end
   end
 end

spec/ruby/core/module/refine_spec.rb

@@ -731,24 +731,49 @@ def foo
       result.should == "foo"
     end
 
+    it "looks in the refined class from included module" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
+
+      a = Module.new do
+        def foo
+           [:A] + super
+        end
+      end
+
+      refinement = Module.new do
+        refine refined_class do
+          include a
+        end
+      end
+
+      result = nil
+      Module.new do
+        using refinement
+
+        result = refined_class.new.foo
+      end
+
+      result.should == [:A, :C]
+    end
+
     # super in a method of a refinement invokes the method in the refined
     # class even if there is another refinement which has been activated
     # in the same context.
-    it "looks in the refined class even if there is another active refinement" do
-      refined_class = ModuleSpecs.build_refined_class
+    it "looks in the refined class first if called from refined method" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
 
       refinement = Module.new do
         refine refined_class do
           def foo
-            "foo from refinement"
+            [:R1]
           end
         end
       end
 
       refinement_with_super = Module.new do
         refine refined_class do
           def foo
-            super
+            [:R2] + super
           end
         end
       end
@@ -760,7 +785,207 @@ def foo
         result = refined_class.new.foo
       end
 
-      result.should == "foo"
+      result.should == [:R2, :C]
+    end
+
+    it "looks only in the refined class even if there is another active refinement" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
+
+      refinement = Module.new do
+        refine refined_class do
+          def bar
+            "you cannot see me from super because I belong to another active R"
+          end
+        end
+      end
+
+      refinement_with_super = Module.new do
+        refine refined_class do
+          def bar
+            super
+          end
+        end
+      end
+
+
+      Module.new do
+        using refinement
+        using refinement_with_super
+        -> {
+          refined_class.new.bar
+        }.should raise_error(NoMethodError)
+      end
+    end
+
+    it "does't have access to active refinements for C from included module" do
+      refined_class = ModuleSpecs.build_refined_class
+
+      a = Module.new do
+        def foo
+          super + bar
+        end
+      end
+
+      refinement = Module.new do
+        refine refined_class do
+          include a
+
+          def bar
+            "bar is not seen from A methods"
+          end
+        end
+      end
+
+      Module.new do
+        using refinement
+        -> {
+          refined_class.new.foo
+        }.should raise_error(NameError) { |e| e.name.should == :bar }
+      end
+    end
+
+    it "does't have access to other active refinements from included module" do
+      refined_class = ModuleSpecs.build_refined_class
+
+      refinement_integer = Module.new do
+        refine Integer do
+          def bar
+            "bar is not seen from A methods"
+          end
+        end
+      end
+
+      a = Module.new do
+        def foo
+          super + 1.bar
+        end
+      end
+
+      refinement = Module.new do
+        refine refined_class do
+          include a
+        end
+      end
+
+      Module.new do
+        using refinement
+        using refinement_integer
+        -> {
+          refined_class.new.foo
+        }.should raise_error(NameError) { |e| e.name.should == :bar }
+      end
+    end
+
+    # https://bugs.ruby-lang.org/issues/16977
+    it "looks in the another active refinement if super called from included modules" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
+
+      a = Module.new do
+        def foo
+          [:A] + super
+        end
+      end
+
+      b = Module.new do
+        def foo
+          [:B] + super
+        end
+      end
+
+      refinement_a = Module.new do
+        refine refined_class do
+          include a
+        end
+      end
+
+      refinement_b = Module.new do
+        refine refined_class do
+          include b
+        end
+      end
+
+      result = nil
+      Module.new do
+        using refinement_a
+        using refinement_b
+        result = refined_class.new.foo
+      end
+
+      result.should == [:B, :A, :C]
+    end
+
+    it "looks in the current active refinement from included modules" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
+
+      a = Module.new do
+        def foo
+          [:A] + super
+        end
+      end
+
+      b = Module.new do
+        def foo
+          [:B] + super
+        end
+      end
+
+      refinement = Module.new do
+        refine refined_class do
+          def foo
+            [:LAST] + super
+          end
+        end
+      end
+
+      refinement_a_b = Module.new do
+        refine refined_class do
+          include a
+          include b
+        end
+      end
+
+      result = nil
+      Module.new do
+        using refinement
+        using refinement_a_b
+        result = refined_class.new.foo
+      end
+
+      result.should == [:B, :A, :LAST, :C]
+    end
+
+    it "looks in the lexical scope refinements before other active refinements" do
+      refined_class = ModuleSpecs.build_refined_class(for_super: true)
+
+      refinement_local = Module.new do
+        refine refined_class do
+          def foo
+            [:LOCAL] + super
+          end
+        end
+      end
+
+      a = Module.new do
+        using refinement_local
+
+        def foo
+          [:A] + super
+        end
+      end
+
+      refinement = Module.new do
+        refine refined_class do
+          include a
+        end
+      end
+
+      result = nil
+      Module.new do
+        using refinement
+        result = refined_class.new.foo
+      end
+
+      result.should == [:A, :LOCAL, :C]
     end
   end
 

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -877,8 +877,9 @@ protected Object callSuper(VirtualFrame frame, Object[] args) {
             final InternalMethod callingMethod = RubyArguments.getMethod(callingMethodFrame);
             final Object callingSelf = RubyArguments.getSelf(callingMethodFrame);
             final DynamicObject callingMetaclass = metaClassNode.executeMetaClass(callingSelf);
+            final DeclarationContext declarationContext = RubyArguments.getDeclarationContext(frame);
             final MethodLookupResult superMethodLookup = ModuleOperations
-                    .lookupSuperMethod(callingMethod, callingMetaclass);
+                    .lookupSuperMethod(callingMethod, callingMetaclass, declarationContext);
             final InternalMethod superMethod = superMethodLookup.getMethod();
             return callSuperMethodNode.executeCallSuperMethod(frame, callingSelf, superMethod, args, null);
         }

src/main/java/org/truffleruby/core/method/MethodNodes.java

@@ -206,7 +206,7 @@ protected Object superMethod(DynamicObject method) {
             Object receiver = Layouts.METHOD.getReceiver(method);
             InternalMethod internalMethod = Layouts.METHOD.getMethod(method);
             DynamicObject selfMetaClass = metaClassNode.executeMetaClass(receiver);
-            MethodLookupResult superMethod = ModuleOperations.lookupSuperMethod(internalMethod, selfMetaClass);
+            MethodLookupResult superMethod = ModuleOperations.lookupSuperMethod(internalMethod, selfMetaClass, null);
             if (!superMethod.isDefined()) {
                 return nil;
             } else {

src/main/java/org/truffleruby/core/method/UnboundMethodNodes.java

@@ -203,7 +203,7 @@ public abstract static class SuperMethodNode extends CoreMethodArrayArgumentsNod
         protected Object superMethod(DynamicObject unboundMethod) {
             InternalMethod internalMethod = Layouts.UNBOUND_METHOD.getMethod(unboundMethod);
             DynamicObject origin = Layouts.UNBOUND_METHOD.getOrigin(unboundMethod);
-            MethodLookupResult superMethod = ModuleOperations.lookupSuperMethod(internalMethod, origin);
+            MethodLookupResult superMethod = ModuleOperations.lookupSuperMethod(internalMethod, origin, null);
             if (!superMethod.isDefined()) {
                 return nil;
             } else {