[Ironruby-core] Code Review: Better error messages

Shri Borde Shri.Borde at microsoft.com
Sat Jan 24 18:10:41 EST 2009


Agreed, this is a negative scenario and exact error reporting is not a priority in such contrived scenarios.

-----Original Message-----
From: Tomas Matousek 
Sent: Saturday, January 24, 2009 3:03 PM
To: Shri Borde; Jim Deville; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: Better error messages

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.

Tomas

-----Original Message-----

http://ruby-doc.org/core/classes/Array.html
array.fill(obj) → array
array.fill(obj, start [, length]) → array
array.fill(obj, range ) → array
array.fill {|index| block } → array
array.fill(start [, length] ) {|index| block } → array
array.fill(range) {|index| block } → array

MRI
irb(main):005:0> [1,2].fill(1,1,2) {|i| puts "index=#{i}" }
ArgumentError: wrong number of arguments (3 for 2)

IronRuby
>>> [1,2].fill(1,1,2) {|i| puts "index=#{i}" }
=> [1, 1, 1]

Thanks,
Shri


-----Original Message-----
From: Jim Deville 
Sent: Friday, January 23, 2009 3:05 PM
To: Shri Borde; IronRuby External Code Reviewers; DLR Code Reviews
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: Better error messages

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

JD

-----Original Message-----
From: Shri Borde 
Sent: Friday, January 23, 2009 2:04 PM
To: IronRuby External Code Reviewers; DLR Code Reviews
Cc: ironruby-core at rubyforge.org
Subject: Code Review: Better error messages

  tfpt review "/shelveset:error;REDMOND\sborde"

Microsoft.Scripting:

  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?




More information about the Ironruby-core mailing list