Clojurians Log v2

no

generally, we make contracts about what is accepted, and no promises around what happens when you do things outside of the positive contract

also, in general, we never promise concrete exception types

e.g. we might sacrifice a kitten if you give a character to a (vector-of :int)

I know that the deref error message patch (https://clojure.atlassian.net/browse/CLJ-1162) was backed out of 1.12 when it was found datomic relied on the existing exception, but i'm wondering if that's a one-off or something y'all look for in general

it wasn't because it relied on the existing exception

but because Datomic relied on a particular call chain terminating

huh okay

(there was a circularity between .get and .deref)

I work on Datomic, reviewed this IIRC

Well, I'm working on https://clojure.atlassian.net/browse/CLJ-2822 for fun, and I think that changing intCast(Object n) to throw an UnsupportedOperation or IllegalArgument with a better error message would be helpful across the board, but the vector-of tests have given me pause

Most casts in Clojure aren't user-facing, they're implementation details, so imo saying something like "UnsupportedOperation: Expected int, received PersistentVector" is more readable and actually communicates the mismatch between user code and internals than "ClassCastException: Tried to cast PersistentVector to int"

UnsupportedOperation: Expected int, received PersistentVector ClassCastException: Tried to cast PersistentVector to int those don't look substantially different to me.

I realize the CCEs add noisy module info

lol the message can be massaged, i mostly mean that we write something intentional instead of leaving it up to the mechanical CCE

you'll have to clear a very high bar for this

the polymorphic cascade fns in RT are very perf sensitive

this is what i'm thinking, with whatever message makes sense

@@ -1238,7 +1252,9 @@ static public int intCast(Object x){
                long n = longCast(x);
                return intCast(n);
                }
-       return ((Character) x).charValue();
+       if(x instanceof Character)
+               return ((Character) x).charValue();
+       throw new IllegalArgumentException("Expected int, receieved " + x.getClass().getName());
 }

non-starter for performance. There was another ticket that did this and it was walked back

oh?

string concat blows up bytecode size and hurts inlining

enough things are cast from Character to ints that we have to worry about this?

this is in somewhere even more critical than the nth impl in RT.java

intCast is used literally everywhere when interoping with Java

trying to save you wasted effort

this is all great info, i'm very grateful that you're explaining it

i have now been the primary clojure advocate at multiple jobs so i get to both show off clojure and also apologize for clojure a lot

even tho clojure is closer to "source available" than "open source", i remain dedicated to the goal of "making clojure easier to use and debug"

that's why i'm here, working on this

so any knowledge will be helpful for that goal

"passed the wrong things" is a dev time goof that is very low priority to address

sussed out quickly with effective use of the REPL

i've run into this on multiple occasions in datadog logs

sadly, we don't all get the opportunity to work on thoughtful codebases. i mostly work on things written by a succession of folks new to clojure and have to make do with what's available

making errors better is helpful for everyone and shortens both the "easily sussed out" problems at the repl and the "i've spent multiple hours tearing my hair out" problems discovered in o11y logs

spec instrumentation is a column in a decision matrix

(however that conflicts with :inline, which nth is)

yeah if we remove inline, we don't even need spec instrumentation because the stack trace will say "error in clojure.core/nth"

my goal isn't "check inputs of nth", it's "error in a helpful way"

just to +1 stuff Ghadi said above: • in general, undefined behavior is undefined 😜 • if exception isn't mentioned in the docstring, should not consider the throwing of or type of to be contract • clojure tests for errors should not be considered indication of contract behavior • error construction is preferred out of mainline flow of control to support hotspot inlining

what do you mean by that last line?

not sure if my edit helped?

Outline into a separate method: fail(Object arg1)

i'll wait for you to finish typing lol

constructing an error is presumably exceptional behavior, not mainline code and often involves a lot of string/collection work - we usually prefer to move that work into a second method "throwError" that can be called when an error occurs, so that the messy stuff is not in the main code

that makes sense

I'll leave the changes to intCast out of my nth patch

well, if either of you feel like spending some time looking at my work, i'd love the feedback: https://clojure.atlassian.net/browse/CLJ-2822

it's in my 1.13 list so will get to it at some point