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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Copy link
Copy Markdown
Member

@jeaye jeaye Dec 18, 2025

Choose a reason for hiding this comment

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

Sure, sounds good.

Copy link
Copy Markdown
Collaborator

@dgr dgr Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I wrote the original default case for big-int?. I'm looking through the current version of it now and it seems like in some dialects it's becoming a synonym for integer?. I'm not sure that's what we want as it changes the semantics of it. IMO, there are a couple different cases that we need to think through. If a dialect doesn't act like Clojure JVM, there are a couple of choices. The dialect could hard fail on big integers, similar to what ClojureScript does on rationals. This would imply throwing an exception when reading a big integer (e.g., fail on "0N"). Or it could silently incorporate them for those in a given range. Thus, reading "0N" would succeed, but you'd have another kind of number, not a true big integer (again, similar to what JavaScript does). The question is what are the semantics of big-int? in the case of an implementation that just silently converts big integers to another type? In other words, is big-int? semantically equivalent to "Is this a big integer distinct from fixed-precision integers where int? returns true?" or is it semantically equivalent to "Is the type of this object the same type that is the result of (read-string "0N")?" Those are not quite the same thing. It feels like the bit-int? predicate semantics have gotten a little mushy. Specifically, the current code just accepts anything "big integerish" (and even "integerish") in a dialect that doesn't have big integers, and I'm not sure that's what we want. I'd have to go look at all the tests that use the bit-int? predicated to think this through more carefully, but just wanted to highlight the issue.

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.
Comment thread
shantanu-sardesai marked this conversation as resolved.
:default (is (p/big-int? (*' (long (/ r/min-int 2)) 3))))
#?(:jank nil
:default (is (p/big-int? (*' 3 (long (/ r/min-int 2))))))))