Port arithmetic operators for jank#828
Port arithmetic operators for jank#828shantanu-sardesai wants to merge 5 commits intojank-lang:mainfrom
Conversation
|
|
||
| (is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' 1 nil))) | ||
| (is (thrown? #?(:cljs :default :clj Exception :cljr Exception) (+' nil 1))) | ||
| #?(:jank [] |
There was a problem hiding this comment.
Blocked by the jank-lang/jank#558.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
shantanu-sardesai
changed the title
jank
shantanu-sardesai
force-pushed
the
issues/359-auto-promotion
branch
2 times, most recently
from
8c13328 to
3a6d889
Compare
shantanu-sardesai
force-pushed
the
issues/359-auto-promotion
branch
from
e40cecc to
47d24c5
Compare
shantanu-sardesai
force-pushed
the
issues/359-auto-promotion
branch
from
4477c90 to
f7c8103
Compare
shantanu-sardesai
force-pushed
the
issues/359-auto-promotion
branch
from
f7c8103 to
7e2af5e
Compare
| :jank (cpp/jank.runtime.is_big_integer n) | ||
| :default | ||
| (and (integer? n) | ||
| (not (int? n))))) |
There was a problem hiding this comment.
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))There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Agreed, this makes sense. |
4 participants
Adding support for jank's arithmetic promotion operators.