-
Notifications
You must be signed in to change notification settings - Fork 223
Skip instrumenting primitive fns #1176
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.
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
Skip instrumenting primitive fns #1176
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I prefer the warning to an exception as well. Good fix.
Two questions.
docs/function-schemas.md
Outdated
@@ -509,6 +509,8 @@ Turning instrumentation on: | |||
; =throws=> :malli.core/invalid-output {:output [:int {:max 6}], :value 11, :args [10], :schema [:=> [:cat :int] [:int {:max 6}]]} | |||
``` | |||
|
|||
Note that functions with JVM primitive param or return type hints will not be instrumented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the bb exception be mentioned here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know bb won't create a primitive function, but I'm not sure if it can reach vars that contain them. I changed the documentation to be more specific to handle that.
sorry for the delay in merging 😅 |
Closes #859
Throwing seems too disruptive given that it's common to instrument everything.
Other changes:
-find-var
test to the outside so-filter-var
never fails to pass a var-find-ns
set for improved performance