Change in DefaultBinder is to make the binder exception processing
logic be extensible so that languages can customize the type of
exception thrown.
IronRuby:
Improves error reporting in IronRuby by checking the results of the
bind failure. This allowed many Math tests to be enabled.
Also, added a DefaultProtocol for to_f. This allowed more Math tests
to be enabled as you can now pass the string “1.0” to a function
expecting a “Float”. I have added test cases for the Float conversion
rules in core\math\acos_spec.rb. I have not copied this to the other
specs as that would be hard to maintain. We will look at factoring such
conversion checks into a utility library so that they will be much
easier to use.
Fixes some String bugs - String.#inspect on string subtype should
return a string, and passing wrong argument types to Strings#[]= should
cause TypeError instead of ArgumentError. This is where I started and it
put me in the direction of the fixes above.
Tomas: In RubyBinder.GetTypeName, is there a better way to get to
RubyContext?
it “coerces string argument with Float() without calling singleton to_f”
do
s = MathSpecs.string_with_singleton_to_f(“0.5”, 0.5)
Math.acos(s).should be_close(Math.acos(0.5), TOLERANCE)
ScratchPad.recorded.should == nil # to_f should not be called
end
it “coerces string argument with Float() without calling String#to_f”
do
MathSpecs.hijack_String_to_f
begin
Math.acos(“0.5”).should be_close(Math.acos(0.5), TOLERANCE)
ensure
MathSpecs.reset_String_to_f
end
end
These can be rewritten with mocks instead of specialized classes.
it “coerces string argument with Float() without calling singleton to_f”
do
s = “0.5”
s.should_not_receive(:to_f)
Math.acos(s).should be_close(Math.acos(0.5), TOLERANCE)
End
The second spec is really testing method dispatch, and otherwise is the
same thing as this one. Same with:
it “raises an ArgumentError if the singleton string cannot be directly
coerced with Float()” do
s = MathSpecs.string_with_singleton_to_f(“test”, 0.5)
lambda { Math.acos(s) }.should raise_error(ArgumentError)
ScratchPad.recorded.should == nil # to_f should not be called
end
it “raises an ArgumentError if the string subclass cannot be directly
coerced with Float()” do
s = MathSpecs.string_subclass_with_to_f(“test”, 0.5)
lambda { Math.acos(s) }.should raise_error(ArgumentError)
ScratchPad.recorded.should == nil # to_f should not be called
End
Jim, using should_not_receive sounds like a good idea. Will do the
change.
But we still need all those specs including the one using
hijack_String_to_f. It is not testing method dispatch. Ruby method
dispatch only controls which method body will be called. It does not do
argument conversions. That is handled by the target method which is free
to use to_f to convert the argument to a float, but is not required to.
The specs below will ensure that the library method does follow the
right convention/protocol for converting the argument to a float. Does
that make sense?
Also, I had this comment:
///
/// An overload with a block should get precedence. However,
Microsoft.Scripting.Action.Calls.MethodBinder
/// gives precedence to overloads with a matching number of arguments as
the actual arguments. Consider Object#eval
/// which effectively has the following two overloads in the Ruby sense.
/// obj.instance_eval(string [, filename [, lineno]] ) => obj
/// obj.instance_eval {| | block } => obj
/// Consider this call:
/// obj.instance_eval(2, 3) { }
/// This should match with the second Ruby overload, and report an
ArgumentError because of the mismatch in the number of arguments.
/// However, the MethodBinder will prefer the first Ruby overload as it
has a matching number of arguments, and will report
/// a type-mismatch instead.
/// Filterting down the overload list upfront yields better errors.
///
Tomas pointed out that this filtering was incorrect. Tomas, it did cause
test failures (My last test pass had this filtering done in
error-handling after the MethodBinder did its stuff).
However, overloads do get precedence in some cases like the
instance_eval example in the comment above, and also with Array#fill as
shown below. There is a RubySpec test for Array#fill, but we have a tag
because we get it wrong. I agree with Tomas that this will need to be
handled more carefully, and we might need to have an attribute on the
library methods indicating whether the block gets precedence, or whether
the argument count gets precedence in selecting the method overload. I
will undo my change for now.
The question is whether we need to copy the exact error reporting for
argument mismatch, or whether best effort approach isn’t good enough.
The advantage of the current behavior is a simpler binder. We can
certainly add attributes that will prioritize some overloads but it
would make the binder more complicated. I don’t see a scenario where it
makes sense for an app to catch ArgumentErrors and depend on it.
Array#fill behavior might also be considered a bug in MRI.
I believe Jim is right. Library methods performing conversions (or other
operations) do so either by calling internal C methods or using regular
Ruby method dispatch. The former can’t be detected from Ruby except for
using a tracing proc (set_trace_func). Method dispatch goes to the
singleton of the object first and then to its class. So the use of
should_not_receive ensures that to_f is not invoked on the given object.
The test below actually shows that the singleton to_f is not called.
Similarly, a string like “1.0” gets converted to a Float even after
undef-ing String#to_f. So the library methods are not calling to_f on
strings at all, but are directly converting the string contents to a
Float value.
The question is not whether should_not_receive ensures that to_f is not
invoked (I agree that we should use this API). The question is whether
we need the testing of all these cases for the default protocol for
converting Float arguments, or if its redundant with language tests
testing method dispatch. The language tests will cover the conversions
done in the libraries.
Tomas, correct me if I’m wrong, but Ruby uses a message based method
dispatch. When to_f is called, the message to_f gets sent to the object,
and then normal method resolution occurs. There is no way to bypass that
resolution. You can’t call String#to_f instead of the singleton class’s
to_f. You call to_f on the object, and it sends the message. That is why
I argue those tests are the same. If we have the ability, or the
possibility of something different, then something is wrong with our
method dispatch an method resolution. That should be exposed in the
method tests in Language specs. This spec doesn’t need both the case of
a classes instance method and a singleton instance method.