Issue with refinements in blocks

[chrisseaton]

puts RUBY_DESCRIPTION

class A
end

module Rf
  refine(A) do
    def foo; :rf; end
  end
end

class A
  Activate = Proc.new { using(Rf) }

  new.foo rescue nil

  Activate.call

  p(new.foo)
end

ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [x86_64-darwin20]
:rf

truffleruby 21.2.0-dev-09683f1e, like ruby 2.7.3, GraalVM CE JVM [x86_64-darwin]
test.rb:19:in `<class:A>': undefined method `foo' for #<A:0x48> (NoMethodError)
Did you mean?  for
	from test.rb:12:in `<main>'

Code is synthetic. Found by @XrXr.

[eregon]

Probably using should go to the closest module body or so, similar to how visibility looks for the module body frame.

I think actually using was never intended to work like that (it should always be strictly lexical, if not it's basically undefined behavior), and I'd think it only works by luck on CRuby. For instance this fails:

$ ruby -e 'def foo; using Module.new; end; foo'
Traceback (most recent call last):
	2: from -e:1:in `<main>'
	1: from -e:1:in `foo'
-e:1:in `using': main.using is permitted only at toplevel (RuntimeError)

But it doesn't matter, we can implement the reported issue behavior. However, I'm not sure if it's intentional it works like this in CRuby, and it seems best to avoid relying on such behavior.

[eregon]

When replacing Activate.call by using(Rf) it works fine on both CRuby and TruffleRuby:

puts RUBY_DESCRIPTION

class A
end

module Rf
  refine(A) do
    def foo; :rf; end
  end
end

class A
  Activate = Proc.new { using(Rf) }

  new.foo rescue nil

  using(Rf) # Activate.call

  p(new.foo)
end

So the difference here is CRuby seems to walk out of blocks to trigger using vs triggering it on the current frame. We could do that too, although I guess nobody uses using in blocks, maybe it should be forbidden for clarity (since its effect "escapes" the block).

[XrXr]

Just wanted to add a bit to this following recent discussion about cache design for refinements with Benoit. I have another example where TruffleRuby deviates, this one with no blocks surrounding the using call:

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
for refinement in [M1, M2] # could do the same idea with a while loop too
  e.test
  using refinement
  e.test
end

truffleruby 22.3.1, like ruby 3.0.3, GraalVM CE Native [aarch64-darwin]
Example#test
Example#test in M1
Example#test
Example#test in M1
===========
ruby 3.2.1 (2023-02-08 revision 31819e82c8) [arm64-darwin22]
Example#test
Example#test in M1
Example#test in M1
Example#test in M2

The code is a modified version of https://bugs.ruby-lang.org/issues/14083

Refinements went into CRuby way before my time. Until recent chat with Benoit I didn't know that they were designed with the intention that the first lookup within a using scope always gives the right answer. Because of cases like this and the one in the OP, I had assumed that the implementation always needs to lookout for the possibility of someone reentering the scope and calling using 😅 (that doesn't work in all cases even in CRuby, but I'd assumed those were bugs)

[eregon]

Thanks, I forgot about that one. I replied on the CRuby issue. for ... in is less clear than .each because IIRC CRuby uses the same frame outside and inside the for (I'm not entirely sure, would be good to have this confirmed) but this is not the case on TruffleRuby, where for ... in is basically the same .each. Though in this case it doesn't seem to change anything about the output.

So TruffleRuby's behavior here (unrefined, M1, unrefined, M1) is expected according to my understanding of the design of refinements (explained in that reply).

and the one in the OP

Actually the OP implemented parts of refinements on TruffleRuby, so it's more of a synthetic case than something used in real world code.

[XrXr]

There are other hairy cases, like using after retry, using from another thread, etc. I wish we could go back in time and make them a keyword resolved at parse time so we don't need to go up against Rice's just to read the code 😢

[eregon]

Right, but I think we could change semantics for such edge cases in CRuby as needed because they're extremely unlikely used in practice, and it would make sense from a performance POV. Ideally we would raise an exception rather than exhibit strange-looking behavior though.

[XrXr]

Yeah. I haven't thought too deeply about this, but I think making each using call site good for one use cuts off most exploits and makes the system give feedback (via exceptions) for people playing around that haven't fully internalized the semantics. And then there is the problem of handling "suspended" using like in the Proc example. I guess it should be possible to raise in those cases too, by inspecting cache state of all sites affected by that using.