prism icon indicating copy to clipboard operation
prism copied to clipboard

RationalNode numerator & denominator fields to make it directly usable

Open eregon opened this issue 1 year ago • 0 comments

Rational literals are either:

  • 123r
  • 12.34r

(12ri/12.34ri is just ImaginaryNode of RationalNode, so a trivial variant)

These are actually quite different, the first is the same as Rational(123, 1) but the second is Rational(1234, 100), not Rational(12.34):

irb(main):001:0> 12.34r
=> (617/50)
irb(main):002:0> Rational(1234, 100)
=> (617/50)
irb(main):003:0> Rational(12.34)
=> (3473401212609495/281474976710656)

So right now to handle this we have to manually parse the number, e.g. TruffleRuby does it like this: https://github.com/oracle/truffleruby/blob/5096dddf6c3adc7edbae0938efc4fb4913b62bba/src/main/java/org/truffleruby/parser/YARPTranslator.java#L2905-L2927

It would be great if RationalNode was changed to have a numerator and denominator field, then we wouldn't need to manually reparse the number. It could also potentially save an unnecessary conversion from string to double, since the double value in this case should not be used anyway.

code before after
123r RationalNode(numeric=IntegerNode(123)) RationalNode(numerator=IntegerNode(123), denominator=IntegerNode(1) (or nil))
12.34r RationalNode(numeric=FloatNode(12.34)) RationalNode(numerator=IntegerNode(1234), denominator=IntegerNode(100))

cc @andrykonchin

eregon avatar Feb 28 '24 17:02 eregon