Skip to content

Port arithmetic operators for jank #828

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.

Sign up for GitHub

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

Open
shantanu-sardesai wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Port arithmetic operators for jank #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f5594c1
Update number ranges with C++ values
shantanu-sardesai Nov 25, 2025
3a6d889
Port `+'`
shantanu-sardesai Nov 25, 2025
47d24c5
Port `*'`
shantanu-sardesai Nov 25, 2025
1a5a346
Remove redundant `include`
shantanu-sardesai Nov 26, 2025
7e2af5e
Implement `big-int?` for jank
shantanu-sardesai Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions test/clojure/core_test/number_range.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
(ns clojure.core-test.number-range)

;; TODO jank support blocked on https://github.com/jank-lang/jank/issues/195

(def ^:const max-int #?(:clj Long/MAX_VALUE
:cljr Int64/MaxValue
:cljs js/Number.MAX_SAFE_INTEGER
:jank (cpp/value "std::numeric_limits<jank::i64>::max()")
:default 0x7FFFFFFFFFFFFFFF))

(def ^:const min-int #?(:clj Long/MIN_VALUE
:cljr Int64/MinValue
:cljs js/Number.MIN_SAFE_INTEGER
:jank (cpp/value "std::numeric_limits<jank::i64>::min()")
:default 0x8000000000000000))

(def ^:const all-ones-int #?(:cljs 0xFFFFFFFF
Expand All @@ -24,10 +24,12 @@
(def ^:const max-double #?(:clj Double/MAX_VALUE
:cljr Double/MaxValue
:cljs js/Number.MAX_VALUE
:jank (cpp/value "std::numeric_limits<jank::f64>::max()")
:default 1.7976931348623157e+308))

(def ^:const min-double #?(:clj Double/MIN_VALUE
:cljr Double/Epsilon ; NOTE: definitely not Double/MinValue -- ouch!
:cljs js/Number.MIN_VALUE
:jank (cpp/value "std::numeric_limits<jank::f64>::min()")
:default 4.9e-324))

29 changes: 15 additions & 14 deletions test/clojure/core_test/plus_squote.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(ns clojure.core-test.plus-squote
(:require [clojure.test :as t :refer [are deftest is]]
[clojure.core-test.number-range :as r]
[clojure.core-test.portability #?(:cljs :refer-macros :default :refer) [when-var-exists]]))
[clojure.core-test.portability :as p #?(:cljs :refer-macros :default :refer) [when-var-exists]]))

(when-var-exists +'
(deftest test-+'
Expand Down Expand Up @@ -70,18 +70,19 @@
6N 1N 5
6N 1N 5N)

(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' 1 nil)))
(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' nil 1)))
#?(:jank []
Copy link
Contributor Author

@shantanu-sardesai shantanu-sardesai Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocked by the jank-lang/jank#558.

@jeaye

I was thinking, maybe it would be better to have a specific issue ticket in jank (Maybe a child of this ticket) dedicated to updating all the Clojure test suite test cases for jank with support for catching specific exception types once the blocking feature is merged and in the meanwhile we merge these blocked PRs (this one, #790, and more incomming). That way we have a reminder to update these test cases and benefit from the plethora of other test cases.

What do you think?

Copy link
Member

@jeaye jeaye Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just see that as part of jank-lang/jank#415. It's not done until jank is running all of the tests we have in this repo.

Copy link
Contributor Author

@shantanu-sardesai shantanu-sardesai Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fine, so we can merge these PRs and just revisit these tests once jank can catch exceptions by type. Till then I can just keep a list of tests (in the comments of the ticket) that need to be updated.

:default [(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' 1 nil)))
(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' nil 1)))])

(is (instance? clojure.lang.BigInt (+' 0 1N)))
(is (instance? clojure.lang.BigInt (+' 0N 1)))
(is (instance? clojure.lang.BigInt (+' 0N 1N)))
(is (instance? clojure.lang.BigInt (+' 1N 1)))
(is (instance? clojure.lang.BigInt (+' 1 1N)))
(is (instance? clojure.lang.BigInt (+' 1N 1N)))
(is (instance? clojure.lang.BigInt (+' 1 5N)))
(is (instance? clojure.lang.BigInt (+' 1N 5)))
(is (instance? clojure.lang.BigInt (+' 1N 5N)))
(is (p/big-int? (+' 0 1N)))
(is (p/big-int? (+' 0N 1)))
(is (p/big-int? (+' 0N 1N)))
(is (p/big-int? (+' 1N 1)))
(is (p/big-int? (+' 1 1N)))
(is (p/big-int? (+' 1N 1N)))
(is (p/big-int? (+' 1 5N)))
(is (p/big-int? (+' 1N 5)))
(is (p/big-int? (+' 1N 5N)))

(is (instance? clojure.lang.BigInt (+' -1 r/min-int)))
(is (instance? clojure.lang.BigInt (+' r/min-int -1)))))
(is (p/big-int? (+' -1 r/min-int)))
(is (p/big-int? (+' r/min-int -1)))))
1 change: 1 addition & 0 deletions test/clojure/core_test/portability.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
;; In CLJS, all numbers are really doubles and integer? and int?
;; return true if the fractional part of the double is zero
#?(:cljs (integer? n)
:jank (cpp/jank.runtime.is_big_integer n)
:default
(and (integer? n)
(not (int? n)))))
Copy link
Collaborator

@E-A-Griffin E-A-Griffin Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to switch to this predicate, I think we could do a better job than this. Personally I think the default should be something like

(= (type 0N) (type n))

Copy link
Contributor Author

@shantanu-sardesai shantanu-sardesai Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure why this is better? As long as we make sure the check in the compiler for jank.runtime.is_big_integer doesn't have a bug.

Copy link
Contributor Author

@shantanu-sardesai shantanu-sardesai Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Are you referring to the :default case? On my mobile it looked like you were talking about the :jank case. I agree that the check for the default case is... a bit convoluted.

Expand Down
35 changes: 19 additions & 16 deletions test/clojure/core_test/star_squote.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(ns clojure.core-test.star-squote
(:require [clojure.test :as t :refer [are deftest is]]
[clojure.core-test.number-range :as r]
[clojure.core-test.portability #?(:cljs :refer-macros :default :refer) [when-var-exists]]))
[clojure.core-test.portability :as p #?(:cljs :refer-macros :default :refer) [when-var-exists]]))

(when-var-exists *'
(deftest test-*'
Expand Down Expand Up @@ -70,20 +70,23 @@
5 1N 5
5 1N 5N)

(is (thrown? #?(:cljs :default :default Exception) (*' 1 nil)))
(is (thrown? #?(:cljs :default :default Exception) (*' nil 1)))
#?(:jank []
:default [(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (*' 1 nil)))
(is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (*' nil 1)))])

(is (instance? clojure.lang.BigInt (*' 0 1N)))
(is (instance? clojure.lang.BigInt (*' 0N 1)))
(is (instance? clojure.lang.BigInt (*' 0N 1N)))
(is (instance? clojure.lang.BigInt (*' 1N 1)))
(is (instance? clojure.lang.BigInt (*' 1 1N)))
(is (instance? clojure.lang.BigInt (*' 1N 1N)))
(is (instance? clojure.lang.BigInt (*' 1 5N)))
(is (instance? clojure.lang.BigInt (*' 1N 5)))
(is (instance? clojure.lang.BigInt (*' 1N 5N)))
(is (p/big-int? (*' 0 1N)))
(is (p/big-int? (*' 0N 1)))
(is (p/big-int? (*' 0N 1N)))
(is (p/big-int? (*' 1N 1)))
(is (p/big-int? (*' 1 1N)))
(is (p/big-int? (*' 1N 1N)))
(is (p/big-int? (*' 1 5N)))
(is (p/big-int? (*' 1N 5)))
(is (p/big-int? (*' 1N 5N)))

(is (instance? clojure.lang.BigInt (*' -1 r/min-int)))
(is (instance? clojure.lang.BigInt (*' r/min-int -1)))
(is (instance? clojure.lang.BigInt (*' (long (/ r/min-int 2)) 3)))
(is (instance? clojure.lang.BigInt (*' 3 (long (/ r/min-int 2)))))))
(is (p/big-int? (*' -1 r/min-int)))
(is (p/big-int? (*' r/min-int -1)))
#?(:jank nil ;; Currently `long` hasn't been ported in jank.
Copy link
Contributor Author

@shantanu-sardesai shantanu-sardesai Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can port long in the same PR as well but didn't know if we wish to do that as well in the same PR... Although if our definition for long is just jank::i64 then I'll go ahead and port it.

:default (is (p/big-int? (*' (long (/ r/min-int 2)) 3))))
#?(:jank nil
:default (is (p/big-int? (*' 3 (long (/ r/min-int 2))))))))