quot and rem are inefficient

Description

The implementation of the quot and rem functions are needlessly complicated. Currently they are:

However all numbers in js are doubles already, so all this is unnecessary:

Notice that "rem" is simply js-mod, and I'm not sure why no one noticed this before. I keep js-mod for now since a lot of code uses it, and if cljs ever grows a number tower the distinction may be important.

Patch attached, which also:

  • Creates a macro version of quot and rem.

  • Updates documentation for quot, rem, js-mod and mod for clarity.

  • Implement fix (private function to round to zero) with ES6 Math.trunc() if available.

Existing quot and rem tests pass, although there could be some better tests of edge cases (negative decimal num or div, NaN and +-Infinity args).

Environment

None

Activity

Show:
Andrea Richiardi
February 15, 2016, 3:02 AM

Patch now applies. I only tested with Nashorn:

Andrea Richiardi
February 15, 2016, 3:02 AM

Patch cleaned up

Mike Fikes
February 21, 2016, 4:11 AM

Successfully ran Andrea's update to Francis's patch through V8, SpiderMonkey, JavaScriptCore, and Nashorn unit tests.

I also manually ran some of the unit tests in bootstrapped ClojureScript built with the patch.

LGTM.

Mike Fikes
February 21, 2016, 4:23 AM

Since this is a low-level numerics update, also ran the unit tests through ChackraCore (successfully).

Mike Fikes
May 26, 2017, 1:29 AM

CLJS-1164-1.patch no longer applies to master

Assignee

David Nolen

Reporter

Francis Avila

Labels

Approval

None

Patch

Code and Test

Priority

Minor