Skip to content
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

-defun macro to flexibly handle arguments (better than cl-defun) #268

Open
alphapapa opened this issue Jul 10, 2018 · 21 comments
Open

-defun macro to flexibly handle arguments (better than cl-defun) #268

alphapapa opened this issue Jul 10, 2018 · 21 comments
Labels
enhancement Suggestion to improve or extend existing behavior

Comments

@alphapapa
Copy link

Wanting more flexible argument handling, I tried to implement this with pcase, but I couldn't get it to match the plist at the end. It was much easier with -let, however I ran into a problem that I'm not sure how to overcome, or if it's even possible to do so.

(defmacro -defun (name arg-patterns &rest body)
  (declare (indent defun))
  (-let ((body-sym (gensym))
         ((arg-patterns defaults) (cl-loop with patterns with defaults
                                           for pattern = (pop arg-patterns)
                                           until (eq pattern :defaults)
                                           collect pattern into patterns
                                           finally return (list patterns (car arg-patterns)))))
    `(defun ,name (&rest args)
       (let ((,body-sym (lambda ()
                          ,@body))
             ,@defaults)
         (or ,@(--map `(-when-let (,it args) (funcall ,body-sym))
                      arg-patterns)
             (user-error ,(concat (symbol-name name) ": Called with invalid args: %s") args))))))

(-defun nuts ((type number &keys :salted salted)
              (type &keys :salted salted)
              (type number)
              (type)
              :defaults ((number 10) (salted t)))
  (list :type type :number number :salted salted))

(nuts 'peanuts)  ;; => (:type peanuts :number 10 :salted t)
(nuts 'peanuts 20)  ;; => (:type peanuts :number 20 :salted t)
(nuts 'peanuts 20 :salted t)  ;; => (:type peanuts :number 20 :salted t)

That all works great, but when I try to set :salted to nil, it doesn't work, because -let doesn't match the plist key's nil value:

(nuts 'peanuts :salted nil)  ;; => (:type peanuts :number 10 :salted t)

If this could be fixed, this could be a really useful way to define functions that accept arguments. It would allow one to avoid manually looping over argument lists.

Any ideas would be appreciated. :)

In case anyone is interested, here's my aborted attempt using pcase:

(defmacro pcase-lambda* (arg-patterns &rest body)
  (declare (indent defun))
  (let ((body-sym (gensym)))
    `(lambda (&rest args)
       (let ((,body-sym (lambda ()
                          ,@body)))
         (pcase args
           ,@(--map `(,it (funcall ,body-sym))
                    arg-patterns)
           (_ (user-error "Called with invalid args")))))))

(setq argh (pcase-lambda* (`(,files ,pred . ,(map something)))
             ;; TODO: default values
             (list :files files :pred pred :something something)))

(funcall argh 'files 'pred 'something 'something)  ;; => (:files files :pred pred :something nil)
@Fuco1
Copy link
Collaborator

Fuco1 commented Jul 11, 2018

-let can destructure nil fine

(-let (((&plist :foo foo) (list :foo nil)))
  foo)

but you are using -when-let which fails if any binding is nil, that's probably the issue.

(-if-let (((&plist :foo foo) (list :foo nil)))
    "yes"
  "no")

@Fuco1
Copy link
Collaborator

Fuco1 commented Jul 11, 2018

Is there any reason why you have multiple signatures

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

I assume that what happens is that it tries to match the first which structurally fits, right? In which case I see where the problem with the keys matching occurs and why it is a problem.

I think we could have a "stricter" version of &plist which would first check with plist-member and if the key exist accept nil as valid binding so that it passes -when-let... only if the binding does not exist it will fail.

@alphapapa
Copy link
Author

but you are using -when-let which fails if any binding is nil, that's probably the issue.

Ah, yes, I see. It seemed like the most natural -let-like alternative to pcase, but I guess it's not quite that simple.

Is there any reason why you have multiple signatures

Well, yes, the point is that the function can accept arguments in any of those formats. cl-defun can't handle a signature like (&optional type (number 10) &key (salted t)), which seems like the natural way to describe it. So being able to specify a signature in -let style would be very useful.

I assume that what happens is that it tries to match the first which structurally fits, right? In which case I see where the problem with the keys matching occurs and why it is a problem.

I don't think that structurally fits. The arg list is ('peanuts :salted nil), which should match the second form, (type &keys :salted salted), right?

I think we could have a "stricter" version of &plist which would first check with plist-member and if the key exist accept nil as valid binding so that it passes -when-let... only if the binding does not exist it will fail.

Yes, that would be great! Would you be interested in implementing that? If not, do you have any pointers? I don't know the Dash internals the way you do...

@Fuco1
Copy link
Collaborator

Fuco1 commented Jul 11, 2018

About the stricter version, that actually might be quite difficult, because the nil check is handled in -if-let not in the internal dash--match function which does the expansion. And -if-let just checks for non nil and that's it.

(I was thinking in dash--match we could add calls to plist-match or similar but that wouldn't help at all)

@alphapapa
Copy link
Author

alphapapa commented Jul 11, 2018

LOL, at the very moment I finished expanding the series of macros and arrived at:

(let
    ((salted
      (when
          (plist-member --dash-source-267-- :salted)
        (plist-get --dash-source-267-- :salted))))
  (if salted
      (progn
        (funcall g1383))))

...I see your message that's way ahead of me. :)

@alphapapa
Copy link
Author

alphapapa commented Jul 11, 2018

If we could make it expand to something like this, maybe that would work? But I don't know where to begin implementing that.

(let ((--dash-source-267-- args))
  (if --dash-source-267--
      (let ((type (pop --dash-source-267--)))
        (if type
            (let ((number (pop --dash-source-267--))
                  (salted-set-p (plist-member --dash-source-267-- :salted)))
              (if (and number
                       salted-set-p)
                  (let ((salted (plist-get --dash-source-267-- :salted)))
                    (progn
                      (funcall g1383)))))))))

@alphapapa
Copy link
Author

alphapapa commented Jul 11, 2018

Well, not exactly on the topic of Dash, but I found a way to implement it with pcase:

(defun plist-p (list)
  (when list
    (cl-loop for (keyword value) on list by #'cddr
             always (keywordp keyword))))

(let ((args '((peanuts :salted nil)
              (peanuts 20 :salted nil)
              (peanuts 20)
              (peanuts))))
  (cl-loop for set in args
           collect (a-list :args set
                           :result (let ((number 10)
                                         (salted t))
                                     (pcase set
                                       (`(,type . ,(and rest
                                                        (guard (plist-p rest))
                                                        (let (map salted) rest)))
                                        (list :type type :number number :salted salted))
                                       (`(,type ,number . ,(and rest
                                                                (guard (plist-p rest))
                                                                (let (map salted) rest)))
                                        (list :type type :number number :salted salted))
                                       (`(,type ,number)
                                        (list :type type :number number :salted salted))
                                       (`(,type)
                                        (list :type type :number number :salted salted))
                                       (_ (user-error "Invalid args")))))))
;; =>
;; (((:args peanuts :salted nil)
;;   (:result :type peanuts :number 10 :salted nil))
;;  ((:args peanuts 20 :salted nil)
;;   (:result :type peanuts :number 20 :salted nil))
;;  ((:args peanuts 20)
;;   (:result :type peanuts :number 20 :salted t))
;;  ((:args peanuts)
;;   (:result :type peanuts :number 10 :salted t)))

Now the issue is transforming a list like:

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

Into the expansion. :)

@basil-conto
Copy link
Collaborator

Sorry, I haven't really been following this thread, but just a quick question: Are plist keywords required to satisfy keywordp under the existing and proposed plist destructuring schemes for dash.el?

@alphapapa
Copy link
Author

@basil-conto Are you talking about #269?

@basil-conto
Copy link
Collaborator

basil-conto commented Jul 11, 2018

@alphapapa I'm asking how opinionated dash.el is on the type of plist keywords, as a general design principle and possibly as an existing restriction in the code.

AFAICT from a quick glance, the PRs relevant to this question are #81, #111, #268, and #269. I haven't looked at -let recently enough to remember off the top of my head its relevant semantics.

@alphapapa
Copy link
Author

alphapapa commented Jul 11, 2018

@basil-conto Your question may be answered by #111 (comment) (at least, with regard to that PR).

@basil-conto
Copy link
Collaborator

basil-conto commented Jul 11, 2018

@alphapapa

Your question may be answered by #111 (comment)

That comment, as well as the following quick test run of #269:

(-let [(&plist :foo x) '(:foo 1 foo 2 x 3)] x) ; => 1
(-let [(&plist 'foo x) '(:foo 1 foo 2 x 3)] x) ; => 2
(-let [(&plist :foo) '(:foo 1 foo 2 x 3)] foo) ; => 1
(-let [(&plist 'foo) '(:foo 1 foo 2 x 3)] foo) ; => 2

both indicate that -let is just using plist-get (as opposed to lax-plist-get or some custom lookup function) for plist destructuring, so any key type under eq will work.

(at least, with regard to that PR).

What prompted my initial question, however, was your definition of plist-p here: #268 (comment)

This suggests that only plists with keywordp keys will be accepted. Is that so, and if so, why?

@alphapapa
Copy link
Author

What prompted my initial question, however, was your definition of plist-p here

My definition of that function has nothing to do with Dash. :)

@basil-conto
Copy link
Collaborator

What prompted my initial question, however, was your definition of plist-p here

My definition of that function has nothing to do with Dash. :)

OK, thanks for clarifying, and sorry about the noise.

@Fuco1 Fuco1 added the enhancement Suggestion to improve or extend existing behavior label Jul 26, 2018
@alphapapa
Copy link
Author

@Fuco1 Do you have any general advice for building lists containing backquotes and unquotes? Or alternatives to doing that in the first place? As you can see in #268 (comment), I figured out a way to make a pcase that works, but creating that pcase programatically seems very difficult due to inserting backquotes and unquotes. After much trial-and-error, I made some progress, but even then I couldn't seem to create a pattern with a dot in it. Maybe there's something obvious I'm missing...

@Fuco1
Copy link
Collaborator

Fuco1 commented Jul 27, 2018

@alphapapa Not really no, I struggle with those myself. They are handled in quite annoying way.

I think it would still be better to maybe invent a notation where we could have just one arg declaration. Ideally -defun would work like -lambda in that it would use the destructuring system of -let.

@alphapapa
Copy link
Author

I think it would still be better to maybe invent a notation where we could have just one arg declaration.

Sorry, I don't understand what you mean.

Ideally -defun would work like -lambda in that it would use the destructuring system of -let.

Yes, definitely.

@Fuco1
Copy link
Collaborator

Fuco1 commented Jul 27, 2018

@alphapapa Well right now you have

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

which is not aligned with -lambda currently. Also I like to follow the convention that adding - prefix to the built-in versions should keep everything working exactly the same so that any lambda can be converted to -lambda without any error.

@yyoncho
Copy link
Contributor

yyoncho commented Feb 14, 2019

@alphapapa @Fuco1 what is the status of this PR? Can we add -defun which works exactly like -lambda as per @Fuco1's commend and eventually something like --defun(?) for what @alphapapa is suggesting?

@Luis-Henriquez-Perez
Copy link

I'm not sure if this helps but, I've written a -defun macro that's based on -lambda by mostly just copying the code from the -lambda. It seems to work.

(defmacro -defun (name match-form &rest body)
  "Macro that allows desctructuring for defun."
  (declare (doc-string 3)
           (indent defun)
           (debug (&define sexp
                           [&optional stringp]
                           [&optional ("interactive" interactive)]
                           def-body)))
  (let* ((docstring (when (stringp (car-safe body)) (car body)))
         (body (if docstring (cdr body) body)))
    (cond
     ((not (consp match-form))
      (signal 'wrong-type-argument "match-form must be a list"))
     ((-all? 'symbolp match-form)
      `(defun ,name ,match-form ,docstring ,@body))
     (t
      (let* ((inputs (--map-indexed (list it (make-symbol (format "input%d" it-index)))
                                    match-form)))
        `(defun ,name ,(--map (cadr it) inputs) ,docstring 
                (-let* ,inputs ,@body)))))))

Also, I would like a more detailed specification of how the pattern matching suggested by @alphapapa will work so that I (and others seeing this issue) could potentially start working on it.

Right now I have a general idea of what it will do, but there are some use cases where I am unsure what the result will be.

For example one question I have is: how will the -defun determine what to do with an argument list like:

((one two three) (four five) (six))

This could be an example of patten matching suggested by alphapapa where -defun can have 3, 2, or 1 argument. However, it could also be a destructuring match form where the function expects 3 arguments: a list of length 3, a list of length 2 and a list of length one.

Another question is should -defun also have optional type checking?
I think that'd be nice to distinguish cases where arguments are of the same length. Though I'm unsure of what syntax should be used to do this.

@ericdallo
Copy link

#336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Suggestion to improve or extend existing behavior
Projects
None yet
Development

No branches or pull requests

6 participants