metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.46k stars 208 forks source link

Instrumentation of fns with primitive type hints fails #859

Open NoahTheDuke opened 1 year ago

NoahTheDuke commented 1 year ago

Found this via a clojure.spec bug:

Cause: Malli replaces var values with instrumented functions that will not work with primitive function interfaces (double and long)

Approach: Take primitive interfaces into account and make them work, or document/fail that instrumentation will not work with these.

noah ~/personal/malli  [master ≡] 
$ clojure -M:test:repl/rebel
nREPL server started on port 38373 on host localhost - nrepl://localhost:38373
[Rebel readline] Type :repl/help for online help info
user=> (require '[malli.core :as m])
nil
user=> (require '[malli.instrument :as mi])
nil
user=> (defn foo [^double val] val)
#'user/foo
user=> (m/=> foo [:=> [:cat double?] any?])
user/foo
user=> (mi/instrument!)
..instrumented #'user/foo
nil

user=> (foo 5.2)
Execution error (ClassCastException) at user/eval16891 (REPL:1).
class malli.core$_instrument$fn__15293 cannot be cast to class clojure.lang.IFn$DO (malli.core$_instrument$fn__15293 is in unnamed module of loader clojure.lang.DynamicClassLoader @1af7f51; clojure.lang.IFn$DO is in unnamed module of loader 'app')
user=> *e
#error {
 :cause "class malli.core$_instrument$fn__15293 cannot be cast to class clojure.lang.IFn$DO (malli.core$_instrument$fn__15293 is in unnamed module of loader clojure.lang.DynamicClassLoader @1af7f51; clojure.lang.IFn$DO is in unnamed module of loader 'app')"
 :via
 [{:type java.lang.ClassCastException
   :message "class malli.core$_instrument$fn__15293 cannot be cast to class clojure.lang.IFn$DO (malli.core$_instrument$fn__15293 is in unnamed module of loader clojure.lang.DynamicClassLoader @1af7f51; clojure.lang.IFn$DO is in unnamed module of loader 'app')"
   :at [user$eval16893 invokeStatic "NO_SOURCE_FILE" 1]}]
 :trace
 [[user$eval16893 invokeStatic "NO_SOURCE_FILE" 1]
  [user$eval16893 invoke "NO_SOURCE_FILE" 1]
  [clojure.lang.Compiler eval "Compiler.java" 7194]
  [clojure.lang.Compiler eval "Compiler.java" 7149]
  [clojure.core$eval invokeStatic "core.clj" 3215]
  [clojure.core$eval invoke "core.clj" 3211]
  [clojure.main$repl$read_eval_print__9206$fn__9209 invoke "main.clj" 437]
  [clojure.main$repl$read_eval_print__9206 invoke "main.clj" 437]
  [clojure.main$repl$fn__9215 invoke "main.clj" 458]
  [clojure.main$repl invokeStatic "main.clj" 458]
  [clojure.main$repl doInvoke "main.clj" 368]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 667]
  [clojure.core$apply invoke "core.clj" 662]
  [rebel_readline.clojure.main$eval3488$repl_STAR___3489 invoke "main.clj" 82]
  [rebel_readline.clojure.main$repl invokeStatic "main.clj" 91]
  [rebel_readline.clojure.main$repl doInvoke "main.clj" 90]
  [clojure.lang.RestFn invoke "RestFn.java" 397]
  [rebel_readline.clojure.main$_main invokeStatic "main.clj" 112]
  [rebel_readline.clojure.main$_main doInvoke "main.clj" 111]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 667]
  [clojure.core$apply invoke "core.clj" 662]
  [rebel_readline.main$_main invokeStatic "main.clj" 6]
  [rebel_readline.main$_main doInvoke "main.clj" 5]
  [clojure.lang.RestFn invoke "RestFn.java" 436]
  [clojure.lang.Var invoke "Var.java" 393]
  [nrepl.cmdline$interactive_repl invokeStatic "cmdline.clj" 402]
  [nrepl.cmdline$interactive_repl invoke "cmdline.clj" 385]
  [nrepl.cmdline$dispatch_commands invokeStatic "cmdline.clj" 486]
  [nrepl.cmdline$dispatch_commands invoke "cmdline.clj" 473]
  [nrepl.cmdline$_main invokeStatic "cmdline.clj" 496]
  [nrepl.cmdline$_main doInvoke "cmdline.clj" 491]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.lang.Var applyTo "Var.java" 705]
  [clojure.core$apply invokeStatic "core.clj" 667]
  [clojure.main$main_opt invokeStatic "main.clj" 514]
  [clojure.main$main_opt invoke "main.clj" 510]
  [clojure.main$main invokeStatic "main.clj" 664]
  [clojure.main$main doInvoke "main.clj" 616]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.lang.Var applyTo "Var.java" 705]
  [clojure.main main "main.java" 40]]}
opqdonut commented 1 year ago

You're totally right. Until we figure out how to properly handle type hints, mi/instrument! should totally emit an error for cases like this. PR welcome!