Fork me on GitHub
#sci
<
2021-07-30
>
phronmophobic04:07:09

I've been thinking about the defrecord/`protocol` problem a bit. If defrecords created an instance of an actual defrecord rather than a map instance, then all you would need to do is extend that type to the protocols you need in both sci and outside of sci. I was thinking something like:

diff --git a/src/sci/impl/records.cljc b/src/sci/impl/records.cljc
index af002d4..69b9913 100644
--- a/src/sci/impl/records.cljc
+++ b/src/sci/impl/records.cljc
@@ -5,6 +5,8 @@
             [sci.impl.utils :as utils]
             [sci.impl.vars :as vars]))
 
+(clojure.core/defrecord SciRecord [])
+
 (defn defrecord [_ _ ctx record-name fields & protocol-impls]
   (let [factory-fn-str (str "->" record-name)
         factory-fn-sym (symbol factory-fn-str)
@@ -43,12 +45,12 @@
          protocol-impls)]
     `(do
        (defn ~map-factory-sym [m#]
-         (vary-meta m#
+         (vary-meta (merge (->SciRecord) m#)
                     assoc
                     :sci.impl/record true
                     :type '~rec-type))
        (defn ~factory-fn-sym [& args#]
-         (vary-meta (zipmap ~keys args#)
+         (vary-meta (merge (->SciRecord) (zipmap ~keys args#))
                     assoc
                     :sci.impl/record true
                     :type '~rec-type))
I tried to test it locally, but I don't know how to call a sci protocol implementation.
(doseq [proto protos]
  (extend SciRecord
    proto
    (into {}
          (for [[sig-key sig-info] (:sigs proto)
                :let [method-name (:name sig-info)
                      v (->> (:method-builders proto)
                             keys
                             (some (fn [v]
                                     (when (= (-> v meta :name)
                                              method-name)
                                       v))))]]
            [sig-key
             (fn [self & args]
               ;; call sci protocol implementation with type of self
               (apply (sci-protocol-implementation (types/type-impl self))
                      args))]))))

borkdude11:07:23

It's something I've been thinking about but I'm not entirely sure how this solves your problem

borkdude11:07:50

Why doesn't the multi-method approach work, apart from having to patch some vars for SCI-specific targets?

borkdude11:07:25

The reason multi-methods are used is so we can define "protocols" dynamically, at runtime, inside SCI. That's where this protocol -> multimethod implementation comes from.

phronmophobic17:07:43

Since the protocols are used heavily, it requires that the original protocol work for both clojure and sci types. I looked into multi-method technique, but it's not clear to me that there's a reliable way to patch the vars. Using a SciRecord type wouldn't change how protocols are implemented in Sci, it just provides a hook to extend clojure protocols to work for sci records. A record with no defined fields is basically just a map, but it does mean that you can extend protocols specifically for all SciRecords in a way that's explicitly supported by clojure. In the example code above, it extends a list of protos to the SciRecord type to then dispatch to sci's sci-protocol-implementation.

borkdude17:07:13

@smith.adriane But why extend all SciRecords, how would you check outside of SCI if the record in question really implemented the protocol?

borkdude17:07:40

I'm not saying it's not a good approach, just like to hear and understand more about the approach

borkdude17:07:16

> but it's not clear to me that there's a reliable way to patch the vars You can make this reliable by adding ^:redef metadata in your lib

phronmophobic18:07:59

I thought I ran into a road block with this, but can't remember what it was. I'll explore this again when I get a chance.

borkdude18:07:04

ok, let's try that first and then we can continue the discussion about alternate implementations

phronmophobic23:07:01

seems to be working so far. I'll test it out some more and report back with the implementation.

borkdude17:07:26

And then patch the vars for the graalvm / sci target

phronmophobic18:07:23

> how would you check outside of SCI if the record in question really implemented the protocol? that's the piece that I couldn't figure out to test locally. My goal was to just call sci's machinery and let it succeed or fail

borkdude22:07:05

If someone's interested in the bb-like thing for node, join #tbd. We've got reagent + ink working! https://github.com/borkdude/tbd#reagent