Add `-defun' #347
Add `-defun' #347
Conversation
`-defun' is like `-lambda', but with destructuring. To implement it, abstract the arglist and form generation code of `-lambda' into separate functions. Additional benefits of the refactoring are: - `-lambda' (and by extension, `-defun') can now accept empty argument lists - `-lambda' (and `-defun') no longer rebinds symbols (fixing the TODO) Various declaration forms should be handled properly in `-defun'. Motivation: when this is merged, `lsp-defun' can be made an alias for `-defun'.
|
I haven't noticed yyoncho's PR... This is going to be awkward. |
|
I believe my PR to be better: it has no code duplication, and it doesn't rebind symbols, so things like (-lamba (foo)) are the same as (lambda (foo)). Is signing a copyright disclaimer enough, and if so, how would I go about doing that? |
|
I'll add support for &optional. |
Yes.
Just email the completed form at https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future to assign@gnu.org; they'll give you further instructions. It's generally a matter of scanning and emailing a scanned document, or in some countries digitally signing a document. See Thanks. |
dev/examples.el
Outdated
| (defexamples -defun | ||
| (progn (-defun example/cdr ((_ . tail)) tail) | ||
| (example/cdr '(a . b))) => 'b | ||
| (progn (-defun example/car ((cur)) cur) | ||
| (example/car '(a . b))) => 'a | ||
| (progn (-defun example/add-cons ((a . b)) | ||
| "Add the `car' and `cdr' of INPUT0." | ||
| (interactive (list (cons 1 2))) | ||
| (+ a b)) | ||
| (command-execute #'example/add-cons)) => 3) |
There was a problem hiding this comment.
Aren't examples limited to a single line? Have you tried regenerating the docs from this?
There was a problem hiding this comment.
I haven't yet tried regenerating the docs - I'll do that later. I see some two-line examples in -setq.
- Add a `-defun' &rest example. - `-defun': add (declare debug), making it debuggable
|
Now we also support &rest and &optional. |
|
TODO: add &key, making |
Refactor `dash--destructure-body' to handle declarations and docstrings itself, meaning that more code can be shared between `-lambda' and `-defun'. Refactor `dash--destructure-body' to add signatures to the docstring of BODY if necessary. This means that docstrings for `-defun' and `-lambda', ... now generate functions with proper signatures. Add `-defmacro', which is like `-defun' but as a macro.
|
Now we have -defmacro, and correct function signatures :). Also, do you think I should implicitly add a signature doc to -lambda, if a docstring is provided? What we have currently: (-lambda (a b)) = (lambda (a b))
(-lambda (a b) "Some docstring...") = (lambda (a b) "Some docstring...") ; no destructuring and signature comment necessary, as we only use symbols in the arglist
(-lambda ((a b)) "") = (-lambda ((a b)) "\n\n(fn ((a b)))") ; Note the auto-generated signature docstring fragment
(-lambda ((a b))) = (-lambda ((a b))) ; no signature docstring, as the user probably doesn't care if they didn't bother to provide a docstringWhat I could do instead is to never touch Other than &key (which may or may not be needed), this PR is ready. |
|
I couldn't write a proper edebug-form-spec for -defun, -lambda, -defmacro's lambda-lists, because I still don't understand them. |
For &as bindings, don't generate input<n> parameters but use the names in the corresponding functions directly. (-lambda ((sym &as &DocumentSymbol :detail?))) -> (lambda (sym) (-let ...))
|
Now I added:
|
dash.el
Outdated
| (defun dash--normalize-arglist (arglist) | ||
| "Make ARGLIST have the form (MATCHERS...). | ||
| If it is a vector, convert it to a single-matcher arglist." | ||
| (if (vectorp arglist) (list (append arglist nil)) arglist)) |
There was a problem hiding this comment.
Can I use cl-coerce here? append nil seems fragile.
There was a problem hiding this comment.
(append x ()) is the canonical way to convert an array x into a list, so I wouldn't worry about it. Also, dash.el tries to avoid relying on cl-lib.
&as bindings would generate paramters for lists already. Now do the same for destructured vectors, too.
|
My changes seem to not have broken anything: I am using my modified dash.el with lsp-mode, a heavy user of |
|
Thanks, I'll have a closer look in the coming days. Let us know how the copyright assignment process goes. |
basil-conto
added
copyright
|
I have sent the E-Mail for the copyright assignment. |
For some reason, using `-let' in `dash--destructure-body' caused compile-errors when running the tests. Use `let*' + manual destructuring instead, fixing the error and making the test suite pass.
dash.el
Outdated
| (list it (intern (format "input%d" it-index)))) | ||
| args)))) | ||
| (if let-bindings | ||
| `(-let ,let-bindings ,@body-forms) |
There was a problem hiding this comment.
Note the -let: previously, -let* was used instead, which might be more efficient as the latter is implemented in terms of the latter. Not doing that might also make my various symbol and &as optimizations for naught. What do you think?
There was a problem hiding this comment.
I don't think I can say something intelligent on this, as I have not yet looked at the code closely.
But re: -let being implemented in terms of -let* - I think that's an implementation detail that we should not base other design decisions on, since -let should ideally be implemented only in terms of itself.
There was a problem hiding this comment.
You need to make sure that input arguments are evaluated in parallel, whether with -let or -let* doesn't matter much.
There was a problem hiding this comment.
That wouldn't really affect the design - -let and -let* can both be used in this position. However, currently -let* is more efficient (both in terms of macro-expansion and less bytecode)::
(-lambda (in (sym &as (a . b)))) ; expands to...
(lambda ; when using `-let*` here
(in sym)
(let*
((--dash-source-20033--
(car sym))
(a
(pop --dash-source-20033--))
(b --dash-source-20033--))))
(lambda ; when using `-let` here
(in sym)
(let
((input0 sym)) ; note the needless variable
(let*
((--dash-source-20039--
(car input0))
(a
(pop --dash-source-20039--))
(b --dash-source-20039--)))))Evaluating the arguments in parallel or not doesn't make a difference here, as all binder-expressions don't reference any previously defined variables.
The trade off here is slightly better readability vs efficiency - -let makes it obvious that this won't be referring to variables defined earlier, but -let* is more efficient, currently.
Perhaps -let should be optimized more instead?
There was a problem hiding this comment.
You would need to construct some kind of graph of dependencies between the bindings. Which I suppose was too much work and I simply leveraged the built-in let to handle it for me. But yes this is a possible optimization. In practice probably insignificant :) But it could be intelectually stimulating :D
You're right about the lambda, there the parallel eval is handled by the runtime when the arguments are passed to the "wrapper lambda".
`-let*' generates less bytecode than `-let', even if otherwise equivalent, because the latter is implemented in terms of the former, with many temporaries. This is exacerbated when using dynamic binding, because all of those variables also need to be bound dynamically. Add a TODO note because this is an implementation detail; `-let' needs to be optimized independently.
No longer generate a redundant nil in `-defun' and `-defmacro', if no docstring is provided and the arglist doesn't use destructuring. This was due to a bug in `dash--destructure-body' only triggered in that case with NODOC=nil.
Use `def-body', as is used in `cl-defun', allowing `-defun' to be debugged.
Before this PR, a `wrong-type-argument' error was issued if the match-form wasn't of appropriate type. Restore this behavior by `signal'ing this way again if the arglist is of incorrect type.
For `lambda', `declare' is ignored. Do the same in `-lambda', allowing a `-lambda' that starts with `declare' to be debugged. To implement this, extend `dash--decompose-defun-body' to take an optional DECLARE? parameter that determines whether `declare' is supported in addition to `interactive'. Note that macros can still be `interactive'. This is consistent with `cl-defmacro', which doesn't put the first `interactive' in `cl-block'.
It would be a waste of time to implement a proper edebug spec for it, since `dash''s destructuring logic is very complex and extensible. In addition, this would make extending `dash''s destructuring logic harder.
The input<n> temporaries used in the generated function's arglists are now uninterned symbols, preventing users of `-defun', `-lambda', ... from accessing them. To implement this, a new intermediate step was added: `dash--parse-arglist' extracts variable bindings from a given arglist, and `dash--destructure-arglist' (formerly `dash--make-arglist') and `dash--destructure-body' now also take the result of the first function. This needed because uninterned symbols need to be communicated to these functions. Additional benefits of the new intermediate step are that `dash--destructure-body' no longer needs to know anything about how the arglist looks like and that there is no part of `dash' needs to know anything about `lambda', `defun', ... keywords.
As of the previous commit, `dash--matcher?' only has one user, `dash--parse-arglist'. Refactor the former to now also do destructuring, yielding a better API and reducing code, as `dash--as-matcher-variable' and `dash--as-matcher-tail' could be eliminated.
`-let' treats [x &as] as destructuring a vector of two elements, `x' and `&as', while `dash--as-matcher?' would treat it as x being destructured like the empty vector. Fix that by constraining vector matchers to be of at least length 3.
nbfalcon
force-pushed
the
feature/-defun
branch
from
94bbcb9
to
83a3c12
Compare
As of shared arglist parsing refactoring, ARGLIST was only used to compute the docstring. Remove DOC, and make ARGLIST in its place imply that a docstring is to be generated.
(command-execute (lambda () (declare) (interactive))) works, so all declarations must be parsed. This function now behaves similar to `macroexp-parse-body', except that the last form is not treated as the body, so that (-lambda () (interactive)) works.
Abstract the common parts of `-defun', `-lambda', etc. into the new function, `dash--defun'.
`help-add-fundoc-usage', from the pre-loaded `help' library, is more robust (it properly handles quotes), so use that.
nbfalcon
force-pushed
the
feature/-defun
branch
from
9ac1487
to
5018e63
Compare
If a `defun' contains only a single string in the body, that string is considered both result and docstring. Do the same for `-defun'. This patch also makes that function parse a `-defun' whose body consists only of strings correctly. Add regression tests.
`dash--normalize-arglist' should wrap them in another list. Add a regression test.
Only 3 examples are shown in the manual, so move the various cons adding examples later since they are not helpful. As such, show an `interactive' example, a Docstring result example and a square bracket args example. Rename `example/add-cons' -> `example/interactive'.
nbfalcon
force-pushed
the
feature/-defun
branch
from
5018e63
to
66f513e
Compare
-defunis like-lambda, but with destructuring. To implement it, abstractthe arglist and form generation code of
-lambdainto separate functions.Additional benefits of the refactoring are:
-lambda(and by extension,-defun) can now accept empty argument lists-lambda(and-defun) no longer rebinds symbols (fixing the TODO)Various declaration forms should be handled properly in
-defun.Motivation: when this is merged,
lsp-defun' can be made an alias for-defun`.Please note the use of
interninstead ofmake-symbol. Using the latter instead caused test-failures.What
git-commit-major-modedo you prefer? I assumed and usedgit-commit-elisp-text-modehereadd examples