Refinement#import_methods must raise ArgumentError when asked to import a
method that was not defined in Ruby code. monoruby raises nothing for the case
ruby/spec exercises, because the module the spec picks — Zlib — is implemented
in Ruby here rather than as a C extension.
Reproduction
core/refinement/import_methods_spec.rb:279:
2)
Refinement#import_methods when methods are not defined in Ruby code
raises ArgumentError when importing methods from C extension FAILED
Expected ArgumentError(/Can't import method which is not defined with Ruby code: Zlib#*/)
but no exception was raised (#<refinement:String@#<Module:0x...>> was returned)
Assessment
Filing this for the record rather than as a straightforward bug, because the
spec's premise does not hold in monoruby:
- The spec is really testing "importing a C-extension method raises". It reaches
for Zlib as a module that is guaranteed to be a C extension in CRuby.
- monoruby has no C extensions (see
doc/c_extention.md
for the design study). Zlib is Ruby, so importing its methods genuinely
succeeds, and succeeding is arguably the correct answer.
So the observable behaviour is defensible, but the guard itself is still worth
having, because it is not Zlib-specific: the same ArgumentError should fire
for any method with no Ruby body — a #[monoruby_builtin] native method, or an
attribute accessor. That case is not covered by the spec and is currently
untested.
What a fix looks like
In import_methods (monoruby/src/builtins/module.rs), reject any source
method whose FuncId does not resolve to an ISeqInfo, with the CRuby message
Can't import method which is not defined with Ruby code: <Module>#<name>.
That makes native builtins and accessors raise, which is right, and leaves the
Zlib example passing because it is Ruby.
The spec example would then still fail on this platform, for the honest reason
that Zlib is not a C extension. If it stays failing it should be tagged with
that rationale rather than left looking like an unimplemented guard.
Refinement#import_methodsmust raiseArgumentErrorwhen asked to import amethod that was not defined in Ruby code. monoruby raises nothing for the case
ruby/spec exercises, because the module the spec picks —
Zlib— is implementedin Ruby here rather than as a C extension.
Reproduction
core/refinement/import_methods_spec.rb:279:Assessment
Filing this for the record rather than as a straightforward bug, because the
spec's premise does not hold in monoruby:
for
Zlibas a module that is guaranteed to be a C extension in CRuby.doc/c_extention.mdfor the design study).
Zlibis Ruby, so importing its methods genuinelysucceeds, and succeeding is arguably the correct answer.
So the observable behaviour is defensible, but the guard itself is still worth
having, because it is not Zlib-specific: the same
ArgumentErrorshould firefor any method with no Ruby body — a
#[monoruby_builtin]native method, or anattribute accessor. That case is not covered by the spec and is currently
untested.
What a fix looks like
In
import_methods(monoruby/src/builtins/module.rs), reject any sourcemethod whose
FuncIddoes not resolve to anISeqInfo, with the CRuby messageCan't import method which is not defined with Ruby code: <Module>#<name>.That makes native builtins and accessors raise, which is right, and leaves the
Zlib example passing because it is Ruby.
The spec example would then still fail on this platform, for the honest reason
that
Zlibis not a C extension. If it stays failing it should be tagged withthat rationale rather than left looking like an unimplemented guard.