rebcabin / masr

Meta ASR: replacement for aging ASDL
MIT License
4 stars 0 forks source link

type-declaration #18

Open rebcabin opened 1 year ago

rebcabin commented 1 year ago

We recommend that type-declaration be changed to print nil rather than () or [] for its null case.

Here is the reasoning:

type-declaration is currently spec'ced as

(s/def ::type-declaration
  (s/nilable ::symtab-id))

meaning that the following are conforming instances:

       (s/valid? ::type-declaration
                 (type-declaration nil))    := true
       (s/valid? ::type-declaration
                 (type-declaration 42))     := true

--show-asr prints () for a null type-declaration. There are two problems with this:

  1. type-declaration is never a collection, so () or [] are not suitable null values for it.
  2. The kosher way to spec this is
(s/def ::type-declaration
  (s/or :null #(or (= % []) (= % ()))
        :id   ::symtab-id))

This is very difficult to deal with because conformance adds the tag :null or :id.

(type-declaration ())
;; => [:null ()]
(type-declaration 42)
;; => [:id 42]

(s/conform ::type-declaration (second (type-declaration ())))
;; => [:null ()]
(s/conform ::type-declaration (second (type-declaration 42)))
;; => [:id 42]

Recursive conformance calls, say for Variable must be polluted with logic for calls of second in multiple places.

certik commented 1 year ago

We can change it. I would collect all these changes and then we'll do the change at once. Note that () and [] are not equivalent in my mind, [a b c] is a syntactic sugar for (vector a b c ), while (a b c) is just an S-expression with "head=a" and two arguments "a" and "b". I was initially against using nil instead of (), but it's fine if it makes the mapping more natural to clojure, it might even make it more readable.

I have a suspicion that there will be more of these, including things like using 'Add instead of Add, and it might be that we decide later that we will use the printout from --show-asr as Clojure "data" that has to be read with some function and transform it to whatever is more natural for MASR. I don't know.

rebcabin commented 1 year ago

(f b c) is indeed an S-expression that can be evaluated, but it is also just data, as in (list f b c). That fact makes macros possible. Macros don't evaluate S-expressions, they rewrite them as lists.

[] and () are kind of like "identical twins," just barely distinguishable. As a weakness, Clojure is very free about translating (vector a b c) into (list a b c) and vice versa behind your back.

You may think you have a list, you pop it into some transformation, and out it comes as a vector, or vice versa. This is admittedly a weak point of the design. It has to do with lazy evaluation and a couple of implementation details like seq, sequable, and a few more. I'm sure it's possible to understand and predict it all. Normally, I just don't worry about it and live with the minor suprises.

rebcabin commented 1 year ago

I am striving to make the printout of --show-asr something that can be evaluated directly in Clojure without an extra reader step or too many macros.

Issues like 'Local versus Local are trivial to solve:

(define Local 'Local)

Fine distinctions between () and [] are more tricky. It's best to treat them as satisfying the spec empty? I think. For most purposes, e.g., (SymbolTable 42 {:x 1, :y 2, :z 3}) is a function call, whereas ['x 'y 'z] is a collection of symbolic constants. Some Clojure functions might stealthily turn it into a lazy list, so it might pop out as (list 'x 'y 'z), satisfying coll?, #(not (empty? %)), seq?, etc., but it will never be interpreted as a function call unless explicitly evaled, so lists are safe as data collections and most of the time fluidly interchangeable with vectors.

rebcabin commented 1 year ago

https://clojure.org/guides/learn/sequential_colls

rebcabin commented 1 year ago

I may find a better solution via this video, which cleared up some things for me

https://lambdaisland.com/episodes/clojure-seq-seqable

rebcabin commented 1 year ago

yup, I am closing this issue with the following. We don't need to change --show-asr. I can handle [] and () as nil:

(s/def ::type-declaration
  (s/nilable ::symtab-id))
;; heavy sugar
(defn type-declaration [ptr]
  (let [td (s/conform ::type-declaration (if (seqable? ptr)
                                           (seq ptr)
                                           ptr))]
    (if (s/invalid? td)
      ::invalid-type-declaration
      td)))
;; ASDL Back-Channel
(tests
 ;; valid examples
  (let [a-valid (Variable 2 'x (Integer 4)
                         nil [] Local
                         [] []  Default
                         Source Public Required
                         false)]
   (s/valid? ::asr-term a-valid) := true
   (s/valid? ::Variable a-valid) := true)
  ;; See https://github.com/rebcabin/masr/issues/18
  (let [a-valid (Variable 2 'x (Integer 4)
                          [] [] Local
                          [] []  Default
                          Source Public Required
                          false)]
    (s/valid? ::asr-term a-valid) := true
    (s/valid? ::Variable a-valid) := true))
rebcabin commented 1 year ago

reopening as documentation reminder as requested by @certik