Bug #18960: Module#using raises RuntimeError when called at toplevel from wrapped script

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 3.1.2p20
• Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN

[shioyama (Chris Salzberg)]

I noticed that this file works when loaded with load, but fails if you pass true (or a module) as the wrap argument.

# using.rb
using Module.new

This works:

load "./using.rb"

This doesn't:

load "./using.rb", true
# raises RuntimeError (main.using is permitted only at toplevel)

I believe the latter should work.

[shioyama (Chris Salzberg)]

• ruby -v set to 3.1.2p20

[shioyama (Chris Salzberg)]

Pull Request: https://github.com/ruby/ruby/pull/6226

[shioyama (Chris Salzberg)]

I'm new to contributing to Ruby, but I haven't gotten a response on this in a couple weeks so wondering if maybe this requires more context.

For reference, here are the docs on the wrap parameter to Kernel#load:

If the optional wrap parameter is true, the loaded script will be executed under an anonymous module, protecting the calling program's global namespace. If the optional wrap parameter is a module, the loaded script will be executed under the given module. In no circumstance will any local variables in the loaded file be propagated to the loading environment.

So in theory if I have a file like this:

# using.rb
using Module.new

then loading it with wrap in the following:

MyModule = Module.new

load "./using.rb", MyModule

should be the same as this:

module MyModule
  using Module.new
end

However, whereas the latter works fine, the former fails with the error reported in the issue.

Note that if I do the same inside of a block to Module.new, like this:

Module.new do
  using Module.new
end

or like this:

Module.new.module_eval do
  using Module.new
end

then I get a different error, RuntimeError: Module#using is not permitted in methods. So "the loaded script will be executed under an anonymous module" in the docs is somewhat misleading since it's not as simple as injecting the code in using.rb into the block to Module.new. In fact, AFAICT there is no way to achieve what load with a wrap option does using inline Ruby. (This I find very interesting actually.)

Regardless though I don't think the error I'm seeing (main.using is permitted only at toplevel) is correct, I think this should work.

@jeremyevans0 (Jeremy Evans) You most recently contributed to the wrap option, what do you think about this? Am I correct to consider this a bug, and this the correct fix?

[jeremyevans0 (Jeremy Evans)]

shioyama (Chris Salzberg) wrote in #note-3:

@jeremyevans0 (Jeremy Evans) You most recently contributed to the wrap option, what do you think about this? Am I correct to consider this a bug, and this the correct fix?

I'm not sure whether this is a bug, but since using is permitted at top-;evel and also permitted in a module definition, it seems logical it should work at top-level with an implicit top-level module. The pull request looks correct to me. Not sure if the spec should use ruby_bug, I would just use ruby_version_is.

[shugo (Shugo Maeda)]

shioyama (Chris Salzberg) wrote in #note-3:

@jeremyevans0 (Jeremy Evans) You most recently contributed to the wrap option, what do you think about this? Am I correct to consider this a bug, and this the correct fix?

main.using should not be called in a method definition even if it's a wrapped script.
With your fix, the following script causes BUG:

excelsior:ruby$ cat using_in_method.rb
# using.rb
def foo
  using Module.new
end

foo
excelsior:ruby$ cat t.rb
load "./using_in_method.rb", true
excelsior:ruby$ ./miniruby t.rb
/Users/shugo/src/ruby/using_in_method.rb:3: [BUG] vm_cref_dup: unreachable
ruby 3.2.0dev (2022-08-22T06:05:24Z shioyama-using_in_.. 08bdfcde78) [x86_64-darwin21]

I'm not sure whether this issue should be considered a bug, but it may be safe not to backport the fix to stable versions.

[shioyama (Chris Salzberg)]

@jeremyevans0 (Jeremy Evans)

I'm not sure whether this issue should be considered a bug, but it may be safe not to backport the fix to stable versions.

Yes, that's what I had originally, in review I changed it to ruby_bug. Happy to put it back.

@shugo (Shugo Maeda)

Ah ok now I see the purpose of that error, thanks.

I'm not sure whether this issue should be considered a bug, but it may be safe not to backport the fix to stable versions.

Sure that makes sense.

It seems like we have two things (1- not allowing main.using to be called in a method definition, and 2- evaluating code in a wrapped script as if it were namespaced in a module) that are mutually incompatible in this case.

[mame (Yusuke Endoh)]

We discussed this issue at the dev meeting. No one objected to correcting this behavior, but the proposed PR is unacceptable because it causes [BUG] as @shugo (Shugo Maeda) said. Could you investigate the issue and update the PR?

[shioyama (Chris Salzberg)]

Thanks! I'll do that and update here once the PR is again ready for review.

[shugo (Shugo Maeda)]

I guess SEGV is caused by using in a method body, which should be prohibited even in a wrapped script.

[shioyama (Chris Salzberg)]

Hmm, ok this is where I originally encountered this problem:

https://github.com/rails/rails/blob/b9493a47e65287f3b135904dbdd511bbf5a5798c/activesupport/lib/active_support/core_ext/enumerable.rb#L285-L290

# Using Refinements here in order not to expose our internal method
using Module.new {
  refine Array do
    alias :orig_sum :sum
  end
}

Although this is somewhat contrived, you can reproduce the problem on the same file as follows:

mod = Module.new
mod::Enumerable = Enumerable

load $LOAD_PATH.resolve_feature_path("active_support/core_ext/enumerable")[1], mod
# main.using is permitted only at toplevel (RuntimeError)

Requiring the same file works as expected:

require "active_support/core_ext/enumerable"
# loads Enumerable extension

Wouldn't the method body issue also be present with require?

[shioyama (Chris Salzberg)]

@shugo (Shugo Maeda)

Actually, it seems the same problem is there with require:

# foo.rb
def foo
  using Module.new
end

foo

require "foo"
# main.using is permitted only at toplevel (RuntimeError)

So I don't really understand why Ruby allows the usage of using at toplevel with require, but not with load. Isn't this a problem in both cases? In both cases you can call using from a method, but with require toplevel usage doesn't raise unless you call from within a method, whereas with load it raises regardless of whether you're calling from a method or not.

[shioyama (Chris Salzberg)]

Oh never mind, I see. My PR results in the error with require but the unreachable BUG with load.

Ok I'll try to find another way to fix it.

[shugo (Shugo Maeda)]

shioyama (Chris Salzberg) wrote in #note-12:

Oh never mind, I see. My PR results in the error with require but the unreachable BUG with load.

Ok I'll try to find another way to fix it.

Yes, RuntimeError should be raised in both cases.

[shioyama (Chris Salzberg)]

@shugo (Shugo Maeda) I've updated my PR and added a spec calling a method which calls using, which fails with the previous code but passes with the updated code. Can you have a look?

[shugo (Shugo Maeda)]

shioyama (Chris Salzberg) wrote in #note-14:

@shugo (Shugo Maeda) I've updated my PR and added a spec calling a method which calls using, which fails with the previous code but passes with the updated code. Can you have a look?

Thank you!

I found another corner case. The following code should be prohibited in a wrapped script:

MAIN = self

module X
  MAIN.send(:using, Module.new)
end

I suggested changes in the following comment:

https://github.com/ruby/ruby/pull/6226#pullrequestreview-1118228759

[shioyama (Chris Salzberg)]

• Status changed from Open to Closed

Applied in changeset git|82ac4a2399516a3ffda750b815c244aad6d38277.

---

Support using at toplevel in wrapped script

Allow refinements to be used at the toplevel within a script that is
loaded under a module.

Fixes [Bug #18960]