Fork me on GitHub
#clojure
<
2021-11-18
>
Eddie05:11:35

I think I may have found a bug in core.match but would appreciate if someone would sanity check me. :) To start, let’s consider some code that is working as expected. x is not a map (much less a map with key :z) so we fall back to the :else.

(let [x [:a 1]]
  (match [x]
    [{:z z}] :a-map
    :else :not-a-map))
=> :not-a-map
If we tweak this code slightly to specify that the match against a map should only contain a certain set of keys an exception is thrown.
(let [x [:a 1]]
  (match [x]
    [({:z z} :only [:x :y :z])] :a-map
    :else :not-a-map))
Execution error (ClassCastException) ...
clojure.lang.PersistentVector cannot be cast to java.util.Map
It seems like when we add the :only something about the match macro generates code that assumes x is a map. My expectation is that the second version of the code would recognize that x is not map and fall back on the :else. Is my expectation wrong? Is my incantation wrong? Is this a bug?

Alex Miller (Clojure team)11:11:23

I don't know much about core.match but you can use macroexpand-1 or macroexpand to explore what the code expands to to see if you can spot the problem, like (macroexpand-1 '<code-above>)

FiVo14:11:12

The problem seems to be that the check is against clojure.lang.ILookup (both implemented by maps and vectors) and then later (.keySet x) is called (only implemented by maps).

FiVo14:11:38

Seems to be these lines https://github.com/clojure/core.match/blob/master/src/main/clojure/clojure/core/match.clj#L1217-L1219. Changed the last bit to clojure.lang.IPersistentMap , tests still pass (in clj). @U7XR2PZFW let me know if you want to file a ticket otherwise I can do it.

Eddie17:11:45

Thanks @UL638RXE2. I'm not familiar with the process of filing a ticket for the core libraries, but I would like to learn. Do you think you could do it and post the links here for me to follow along? If you are busy, I can do some reading and probably figure it out myself.

Alex Miller (Clojure team)18:11:12

All you need to do is put a question on https://ask.clojure.org

Alex Miller (Clojure team)18:11:11

For supplying the patch, need to become a contributor per https://clojure.org/dev/dev

Eddie05:11:47

ask.clojure question has been created. Depending on where the discussion goes there, I will look into creating a patch. https://ask.clojure.org/index.php/11284/bug-when-matching-on-a-vector-with-a-map-clause-that-uses-only

FiVo08:11:16

Looks good, but your code in the post has some issues. I think you are missing triple quotations.

Eddie17:11:58

Good catch, thanks. It looks like triple quotes don't parse the same on ask.clojure as they do in other places. I used the "code sample" button instead and it used indentation which yielded a much more sane result.

Alex Miller (Clojure team)19:11:02

yeah, the editor there is not great

kah0ona09:11:02

Hi folks, I want to add my-first-data-reader to when I start my project. I use a data_readers.clj file in the root of my project (under src/clj), and it looks like this {clj-time/clj-time #'my.project.readers/read-clj-time}

kah0ona09:11:29

But I get, when jacking in in CIDER/emacs:

Could not start nREPL server: java.lang.ClassCastException: class clojure.lang.Cons cannot be cast to class clojure.lang.Named (clojure.lang.Cons and clojure.lang.Named are in unnamed module of loader 'app')
	at clojure.core$namespace.invokeStatic(core.clj:1597)
	at clojure.core$data_reader_var.invokeStatic(core.clj:7800)
	at clojure.core$load_data_reader_file$fn__8787.invoke(core.clj:7818)
	at clojure.core.protocols$iter_reduce.invokeStatic(protocols.clj:49)
	at clojure.core.protocols$fn__8125.invokeStatic(protocols.clj:75)
	at clojure.core.protocols$fn__8125.invoke(protocols.clj:75)
	at clojure.core.protocols$fn__8073$G__8068__8086.invoke(protocols.clj:13)
	at clojure.core$reduce.invokeStatic(core.clj:6828)
	at clojure.core$load_data_reader_file.invokeStatic(core.clj:7813)
	at clojure.core$load_data_reader_file.invoke(core.clj:7804)

p-himik09:11:59

The values have to be symbols. #'x is expanded into (var x), which is not a symbol.

kah0ona09:11:14

aah check, thanks that was it

👍 1
kah0ona09:11:21

makes sense, too :-)

emil0r10:11:02

What options are there in Clojure for C interop?

Ben Sless10:11:43

Requires using panama features

emil0r10:11:49

What are Panama features?

Ben Sless10:11:16

Features only available in incubator state as of java 17

Ben Sless10:11:07

But as you can see in the readme, you get good performance and readable code

emil0r11:11:47

Thank you both 🙂 . Very helpful!

Joshua Suskalo18:11:17

o/ I'm the author of coffi. If you can't use JDK 17, I have links in the project to several other ffi libraries, including dtype-next and tech.jna, which you could consider using instead.

Sam Ritchie13:11:56

hey all - is there some built-in way to temporary install a multimethod dispatch function for some dispatch value? like with-methods

Sam Ritchie13:11:14

I see that I have the tools to write a function for this, just looking for some prior art!

delaguardo14:11:51

There are addMethod and removeMethod methods (lol) of MultiFn. So you can write a macro to add one execute the body and remove aftafterward. https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/MultiFn.java#L58

delaguardo14:11:54

However i don't think it is a bright idea to use them anywhere outside of testing context

p-himik14:11:45

addMethod and co should not work correctly in a mutlithreaded environment. If you control the multimethod, perhaps something like this would work:

(defn ^:dynamic f [& args] ...)

(defmethod multi-f :x [& args]
  (apply f args))

(binding [f (fn [& args] ... some other implementation ...)]
  (multi-f :x ...))

Sam Ritchie14:11:59

Yup just for testing

delaguardo14:11:27

Another option is to change :default method adding extra dispatch step

emccue15:11:37

What is your multimethod dispatching on, if i can ask?

potetm15:11:57

I would say either: 1. add more defmethods just for test cases (assuming you can feed the dispatch value through, or 2. alter-var-root

potetm15:11:46

I would find a fn like with-methods confusing because it implies a scope (a la with-bindings) but it’s global.

Sam Ritchie15:11:04

hey, back at the computer, yes, let me make clearer why this came up

Sam Ritchie15:11:44

I wanted to give users the ability to override Clojure’s default printer for AFunction and MultiFn, to use a cleaner name if it happened to be attached to the metadata. so + would print as +… cute, but a nice feature when you start to use function algebra in sicmutils, where “adding” functions f and g means, “make a new function that will pass its arg to each of f and g then add the result. So that function would print as (+ f g)

Sam Ritchie15:11:11

or, with derivatives, (+ (* f (D g)) (* g (D f))) etc etc.

Sam Ritchie15:11:15

(defn install-fn-printers!
  "NOTE: If you are using Emacs + cider-nrepl, these two `print-method`
  implementations will not be installed. I'm currently working on a fix to get
  this enabled in the Emacs environment... But if you want to test this feature
  out, simply execute these forms.
  
  TODO write a with-methods instead to do this..."
  []
  #?(:clj
     (do
       (defmethod print-method AFunction [f ^java.io.Writer w]
         (.write w (.toString ^Object (v/freeze f))))

       (defmethod print-method MultiFn [f ^java.io.Writer w]
         (.write w (.toString ^Object (v/freeze f)))))

     :cljs
     (extend-protocol IPrintWithWriter
       MetaFn
       (-pr-writer [x writer _]
         (write-all writer (.toString (v/freeze x))))

       MultiFn
       (-pr-writer [x writer _]
         (write-all writer (.toString (v/freeze x)))))))

Sam Ritchie15:11:31

so I wrote this, and wanted to test that it worked without actually overriding the printers for the entire test suite

Sam Ritchie15:11:29

(another wrinkle was that cider-nrepl already overrides print-method for these types to give better names, but their override battles with mine! Which is why I stuck this into a function, so a user could call it at the REPL. Putting it in a source file probably will not work since cider-nrepl will override.