Skip to content

Commit

Permalink
[Truffle] Fixnum#fdiv specs all passing.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Jan 12, 2015
1 parent 8c74bc3 commit 7d97f4a
Showing 7 changed files with 86 additions and 1 deletion.
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/truffle/nodes/core/FixnumNodes.java
View file
Original file line number Diff line number Diff line change
@@ -640,6 +640,7 @@ public int div(@SuppressWarnings("unused") long a, @SuppressWarnings("unused") R
// TODO(CS): not entirely sure this is correct
return 0;
}

}

@CoreMethod(names = "%", required = 1)
10 changes: 10 additions & 0 deletions core/src/main/java/org/jruby/truffle/nodes/core/FloatNodes.java
View file
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@
import com.oracle.truffle.api.utilities.ConditionProfile;
import org.jruby.truffle.nodes.dispatch.CallDispatchHeadNode;
import org.jruby.truffle.nodes.dispatch.DispatchHeadNodeFactory;
import org.jruby.truffle.runtime.DebugOperations;
import org.jruby.truffle.runtime.RubyCallStack;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.control.RaiseException;
@@ -254,6 +255,15 @@ public double div(double a, RubyBignum b) {
return a / b.doubleValue();
}

@Specialization(guards = {
"!isInteger(arguments[1])",
"!isLong(arguments[1])",
"!isDouble(arguments[1])",
"!isRubyBignum(arguments[1])"})
public Object div(double a, Object b) {
return DebugOperations.send(getContext(), a, "redo_coerced", null, getContext().getSymbolTable().getSymbol("/"), b);

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jan 12, 2015

Author Contributor

@eregon @nirvdrum

We've got the problem that methods like Float#/ have to handle classes such as Rational and Complex on the RHS, but these types don't exist in our Java code. Kevin looked at writing a guard that looks for the type, having read it out of the object graph and then calls a helper method, but I think I've found a better way here.

The Rubinius implementation of Fixnum#/ runs a primitive, and if that fails it does redo_coerced. Their primitive handles the same types we do in our specialisation, so we can just re-use that redo_coerced! We just need to call it if none of our specialisations match.

But I shouldn't have left this as a DebugOperation.send - I'll go back and add a proper call.

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Jan 12, 2015

Member

Is it because it is not easy to express efficiently obj.is_a? Rational as a guard?
I think in this case we just need to implement #coerce correctly and it might indeed be easier in Ruby.
This is going to cause a (tail) recursive call on this node though (with a different specialization hopefully).

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Jan 12, 2015

Author Contributor

We can write that guard, but with Rational and Complex we'll end up doing it a few times, and then each one will have to dispatch to some other method, maybe a helper if things aren't quite compatible. This way is easy - anything we don't handle, go into the Rubinius fallback code. We might want to change for performance later on, but this is helping me get the specs passing without much work.

This comment has been minimized.

Sign in to view

Copy link
@nirvdrum

nirvdrum Jan 12, 2015

Contributor

We have the guard for Rational, but it's more complex than an instanceof check because we only have a handle to a RubyClass for any of the Rubinius classes.
}

}

@CoreMethod(names = "%", required = 1)
8 changes: 8 additions & 0 deletions core/src/main/ruby/jruby/truffle/core/rubinius/kernel/common/fixnum.rb
View file
Original file line number Diff line number Diff line change
@@ -33,4 +33,12 @@ def coerce(other)
super other
end

def fdiv(n)
if n.kind_of?(Fixnum)
to_f / n
else
redo_coerced :fdiv, n
end
end

end
5 changes: 5 additions & 0 deletions core/src/main/ruby/jruby/truffle/core/rubinius/kernel/common/float.rb
View file
Original file line number Diff line number Diff line change
@@ -28,6 +28,11 @@

class Float < Numeric

def coerce(other)
return [other, self] if other.kind_of? Float
[Float(other), self]
end

def to_r
f, e = Math.frexp self
f = Math.ldexp(f, MANT_DIG).to_i
10 changes: 10 additions & 0 deletions core/src/main/ruby/jruby/truffle/core/rubinius/kernel/common/kernel.rb
View file
Original file line number Diff line number Diff line change
@@ -42,6 +42,16 @@ def Rational(a, b = 1)
end
module_function :Rational

def Float(obj)
case obj
when String
Rubinius::Type.coerce_string_to_float obj, true
else
Rubinius::Type.coerce_object_to_float obj
end
end
module_function :Float

##
# MRI uses a macro named StringValue which has essentially the same
# semantics as obj.coerce_to(String, :to_str), but rather than using that
52 changes: 52 additions & 0 deletions core/src/main/ruby/jruby/truffle/core/rubinius/kernel/common/numeric.rb
View file
Original file line number Diff line number Diff line change
@@ -28,6 +28,58 @@

class Numeric

#--
# We deviate from MRI behavior here because we ensure that Fixnum op Bignum
# => Bignum (possibly normalized to Fixnum)
#
# Note these differences on MRI, where a is a Fixnum, b is a Bignum
#
# a.coerce b => [Float, Float]
# b.coerce a => [Bignum, Bignum]
#++

def coerce(other)
if other.instance_of? self.class
return [other, self]
end

[Float(other), Float(self)]
end

##
# This method mimics the semantics of MRI's do_coerce function
# in numeric.c. Note these differences between it and #coerce:
#
# 1.2.coerce("2") => [2.0, 1.2]
# 1.2 + "2" => TypeError: String can't be coerced into Float
#
# See also Integer#coerce

def math_coerce(other, error=:coerce_error)
begin
values = other.coerce(self)
rescue
if error == :coerce_error
raise TypeError, "#{other.class} can't be coerced into #{self.class}"
else
raise ArgumentError, "comparison of #{self.class} with #{other.class} failed"
end
end

unless Rubinius::Type.object_kind_of?(values, Array) && values.length == 2
raise TypeError, "coerce must return [x, y]"
end

return values[1], values[0]
end
private :math_coerce

def redo_coerced(meth, right)
b, a = math_coerce(right)
a.__send__ meth, b
end
private :redo_coerced

def zero?
self == 0
end
1 change: 0 additions & 1 deletion spec/truffle/tags/core/fixnum/fdiv_tags.txt
View file

This file was deleted.

0 comments on commit 7d97f4a

Please sign in to comment.