[ruby-core:112642] [Ruby master Bug#14083] Refinement in block calling incorrect method - ruby-core - ml.ruby-lang.org

1 Mar 2023

Issue #14083 has been updated by Eregon (Benoit Daloze).

My reading of
https://bugs.ruby-lang.org/projects/ruby-master/wiki/RefinementsSpec#Scope-…
is that the expected output here is:
...
  A refinement is activated in a certain scope.The scope
of a refinement is lexical in the sense that, when control is transferred outside the
scope (e.g., by an invocation of a method defined outside the scope, by load/require,
etc...), the refinement is deactivated.In the body of a method defined in a scope where a
refinement is activated, the refinement is activated even if the method is invoked outside
the scope. 
Given the scope is limited by `class`/`module` keywords like constant scopes
(https://bugs.ruby-lang.org/issues/11779#note-31), it should be:
```
Example#test
Example#test in M1
Example#test
Example#test in M1
```
which is what TruffleRuby does currently.
Seeing both M1 and M2 from the same call site wouldn't be lexical.

The reason we don't see M2 there is changing the refinements for a scope after there
have been calls is AFAIK an incorrect usages of refinements (ideally `using` would raise
for such a case, but it might be difficult to detect).
In other words, refinements at a given call site must always be the same and so it's
enough to consider refinements during the initial lookup for the cache and not after.
This is the key point after the very long discussion on the mailing list about the
original design of refinements (at least that's what I recall from it), they must not
have dynamic rebinding so they don't have extra cost (e.g. on non-refined method calls
when in a scope with some refinements activated, and also obviously nobody wants to check
if there are active refinements at every call site), the refinements for a given call site
should be fixed and never change. If they change, it's an incorrect usage and it
should be fair to just ignore the change (what TruffleRuby does, or even better to raise
an error in that case).

In practice, I believe real usages of `using` are only early at the top-level, much like
`require`, and maybe sometimes at the beginning of a `module`/`class` body.
Both of these are fine and can't run into this problem (well, except if they meant to
`refine` `require` but then it's only natural to call `using` before `require`).

@shugo What do you think about this?
I think we should try to raise an error for `using` in such invalid cases.
That would make it much easier to fix #18572 on CRuby and allow simplifying the
implementation of refinements on CRuby (e.g., TruffleRuby doesn't track if a method
has refinements).

----------------------------------------
Bug #14083: Refinement in block calling incorrect method
https://bugs.ruby-lang.org/issues/14083#change-102089

* Author: bjfish (Brandon Fish)
* Status: Rejected
* Priority: Normal
* Backport: 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Tested on ruby versions 2.3.4 and 2.4.1

When a refinement is used inside a block, the scope of the refinement is not ending when
the block has ended. The following example illustrates the issue:

Example:

~~~ ruby
class Example
  def test
    puts "Example#test"
  end
end

module M1
  refine Example do
    def test
      puts "Example#test in M1"
    end
  end
end

module M2
  refine Example do
    def test
      puts "Example#test in M2"
    end
  end
end

e = Example.new
[M1, M2].each { |r| 
  e.test
  using r
  e.test 
}
~~~

Actual Output

~~~ text
Example#test
Example#test in M1
Example#test in M1
Example#test in M2
~~~

Expected output

~~~ text
Example#test
Example#test in M1
Example#test
Example#test in M2
~~~

-- 
https://bugs.ruby-lang.org/