Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interpreter: represent all numbers as rationals #119

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Mar 4, 2025

Summary

  • implement proper support for rational numbers in the meta-language
    interpreter
  • fix non-integer FloatVal not being considered values in the source
    language definition
  • significantly improve compile times of language and term macro
    invocations

Details

  • add the Rational type, implemented as a numerator/denominator pair
  • implement the basic arithmetic and comparison operators for Rational
  • store all numbers as Rationals in Node. All usage sites of Node
    expecting numbers stored as strings in .sym are updated
  • update z pattern matching in the interpreter to only match integer
    numbers
  • remove newLit usage from language and term; newLit cannot be
    used anymore, as the fields of Rational (stored in Node) are not
    exported. The actual processing is factored into normal procedures
    taking a NimNode as input, with the macros then emitting a call to
    the respective procedures, passing the quoted macro argument along
  • use rat (instead of type variables) as the parameter type for some
    built-in functions, to improve type checking precision and type
    inference
  • fix a stack overflow crash with divMod, due to low(Int128) not
    being considered

The newLit usage majorly contributed to compile times, because:

  1. the newLit call itself has to process a large amount of data
  2. the (potentially huge) construction expressions emitted by the
    macros need to be sem-checked again and - if appearing in a
    const context - compiled for and evaluated with the VM

Removing the newLit usage therefore significantly decreases compile
times.


To-Do

  • narrow down the built-in numeric operations' types
  • add tests (requires some unit test infrastructure first)
  • write a proper commit message

@zerbina zerbina added the enhancement New feature or request label Mar 4, 2025
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 5, 2025

To not lose information for rationals that don't have a (finite) string representation, Node must store numbers as Rationals directly. This is a bit more effort to implement, but the upside is that it'll get rid of the repeated parse/stringify operations in the interpreter.

zerbina added 16 commits March 6, 2025 20:30
Move the implementation of the `languages` macro into a procedure and
emit a call to that procedure from the `languages` macro.

This addresses two problems:
1. it eliminates the very high compile-time overhead (~1.5 seconds for
   `source.nim`) to that `newLit` + sem + evaluate has
2. it doesn't require a manual implementation of `newLit` for
   `Rational` and `Int128` (both whose fields are not exported)
All processing of numbers, which previously accessed the `.sym` field,
are changed to use `.num`. The `rationals` API is extended with some
conversion-related procedures.
Integer-type patterns now only match for integer values, as they
should.
Concrete function are faster to type check, and this also prevents bugs
by passing unsupported values to the arithmetic operators.
The operator is named `div` now.
It's used to conveniently created fractions (both proper and improper).
The resulting denominator must always be positive.
It still assumed that rational numbers were stored as strings.
`newLit` cannot be used with `Node` anymore. The same solution as with
the `lanugage` macro is used, that is, actual processing/production is
factored into its own procedure, which the macro then emits a call to.
Only `FloatVal` with an integer argument was erroneously considered a
value.
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 10, 2025

It would have also been possible to render fractions with no finite decimal number representation as a/b, but this would have slightly complicated both parsing/rendering, while not addressing the repeated parsing/stringification when operating on the numbers in the interpreter.

@zerbina zerbina added the bug Something isn't working label Mar 11, 2025
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 11, 2025

On my machine, the compile time savings are at around 1 second, which is quite good.

zerbina added 3 commits March 12, 2025 19:14
The former two are needed by a fix for rationals, the latter one is
added for completeness' sake.
Due to signed division using recursion and unary `-` for negative
numbers not always resulting in positive values (unless treating the
result as unsigned), `x div low(Int128)` led to infinite recursion.

`divMod` is rewritten to not use recursion, fixing the problem.
The `reduced` implementation had the same issues as `divMod` for
`Int128`, that is, I didn't consider `not isNeg(-x)` doesn't always
hold true when `isNeg(x)`.

The procedure is changed to not use recursion and division is changed
to always be unsigned (because it's faster).
@zerbina zerbina marked this pull request as ready for review March 12, 2025 19:25
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 12, 2025

I've fixed an edge case with signed 128-bit integer division and reduce where they got stuck in an infinite recursion (followed by a crash due to a stack overflow).

@saem: The PR is ready for review now, but please don't merge it yet. I want to first make sure that everything still works after #127 has been merged.

@zerbina zerbina requested a review from saem March 12, 2025 19:29
@zerbina zerbina removed the request for review from saem March 13, 2025 22:45
@zerbina zerbina marked this pull request as draft March 13, 2025 22:45
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 13, 2025

Okay, back to a draft it goes.

Rational numbers (for usage as the numerical value representation in the interpreter) needs arbitrary precision to be practically useful. My previous plan was to first merge rational number support and then implement and use some simple bignum library in post, but I'm almost done with implementing the bignum module already, so there's no point in merging a (mostly) useless rational number library first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant