bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Drew Adams]

1. `macroexp-let2' is used plenty in the Lisp sources now.  And its
documentation is in complete/rudimentary.  Please fix this.

1a. It is not even mentioned in the Elisp manual.  Please document it 
properly there.

1b. The doc string says:

 "Bind VAR to a copyable expression that returns the value of EXP.
  This is like `(let ((v ,EXP)) ,EXPS) except that `v' is a new
  generated symbol which EXPS can find in VAR."

What on Earth does it mean for EXPS to find the symbol `v' in VAR?
VAR is a symbol.  What does it mean for a list of expressions to find
another symbol "in" symbol VAR?  Just what does it mean for the
expression to which VAR is bound to be "copyable" or not "copyable"?
If you mean `macroexp-copyable-p' then say so.

I'm only presuming that EXPS is a list of Lisp sexps.  It is not even
described, but it needs to be.  Nor is EXP described, for that matter.
I'm guessing that it is a Lisp sexp, but it too needs to be specified
properly.

What does it mean for "the `let'" to be skipped"?  How is TEST really
used - what is it for?

So far, this macro description is gobbledygook.  Please document the
macro properly.

2. And then there is `macroexp-let2*', whose doc string says only to
bind each binding (bind a binding?!) "as `macrolet2' does".  That
means nothing.  Presumably, based on the `*' in the name, the behavior
is similar to that of `let*'.  If so, you can use the doc of `let*
as inspiration.

3. The doc strings of `macroexp-let*', `macroexp-progn', and others
say that the function returns "an expression equivalent to" some
expression.  They should say what they mean by equivalence, here.
If you mean that they use `macroexp-quote' to create the
"equivalent" expression then say so.

The doc string of `macroexp-let*' needs to use BINDINGS and EXP, not
the same in lowercase, in the equivalent expression.  Similarly, the
doc string of `macroexp-if' needs to use uppercase TEST, THEN, and
ELSE.

4. The doc string of `macroexp--accumulate' refers to parameters
that do not exist: VAR and LIST.  It should say explicitly (and
not just via `(fn ...)') that the first parameter is a list of
the form `(VAR LIST)'.  (And no, it does not matter that this
macro is internal.)

5. The doc string of `macroexp--cons' is incomprehensible.


In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-11-30 on LEG570
Bzr revision: 3517da701ea5d16c296745d6678988b06bee615d
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --enable-checking=yes,glyphs
 CPPFLAGS=-DGLYPH_DEBUG=1'

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> 1. `macroexp-let2' is used plenty in the Lisp sources now.  And its
> documentation is in complete/rudimentary.  Please fix this.

Looks like Paul added extensive documentation to this macro i 2015.

> 2. And then there is `macroexp-let2*', whose doc string says only to
> bind each binding (bind a binding?!) "as `macrolet2' does".  That
> means nothing.  Presumably, based on the `*' in the name, the behavior
> is similar to that of `let*'.  If so, you can use the doc of `let*
> as inspiration.

Uhm...  I have no idea what it does:

(defmacro macroexp-let2* (test bindings &rest body)
  "Bind each binding in BINDINGS as `macroexp-let2' does."
  (declare (indent 2) (debug (sexp (&rest (sexp form)) body)))
  (pcase-exhaustive bindings
    ('nil (macroexp-progn body))
    (`((,var ,exp) . ,tl)
     `(macroexp-let2 ,test ,var ,exp
        (macroexp-let2* ,test ,tl ,@body)))))

Perhaps somebody could use their decoder ring.

> 3. The doc strings of `macroexp-let*', `macroexp-progn', and others
> say that the function returns "an expression equivalent to" some
> expression.  They should say what they mean by equivalence, here.
> If you mean that they use `macroexp-quote' to create the
> "equivalent" expression then say so.

No, the meaning here seems to be that they are semantically equivalent.
I changed the doc string of macroexp-progn to 

  "Return EXPS with `progn' prepended.
If EXPS is a single expression, `progn' is not prepended."

but I think the rest are OK.

> The doc string of `macroexp-let*' needs to use BINDINGS and EXP, not
> the same in lowercase, in the equivalent expression.

I've now fixed this.

> Similarly, the doc string of `macroexp-if' needs to use uppercase
> TEST, THEN, and ELSE.

Somebody else has fixed that in the intervening years.

> 4. The doc string of `macroexp--accumulate' refers to parameters
> that do not exist: VAR and LIST.  It should say explicitly (and
> not just via `(fn ...)') that the first parameter is a list of
> the form `(VAR LIST)'.  (And no, it does not matter that this
> macro is internal.)

This has been fixed, too.

> 5. The doc string of `macroexp--cons' is incomprehensible.

I've now fixed it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>  (defun macroexp-progn (exps)
> -  "Return an expression equivalent to \\=`(progn ,@EXPS)."
> +  "Return EXPS with `progn' prepended.
> +If EXPS is a single expression, `progn' is not prepended."
>    (if (cdr exps) `(progn ,@exps) (car exps)))

That's described a bit confusingly: AFAIU EXPS should always be a list
of expressions, and when it's _a_list_ of only one expression, `progn'
is not prepended and the expression is returned (and not EXPS as your
text suggests).  BTW, compared to that description I find the original
version much better (simpler).

> > 2. And then there is `macroexp-let2*', whose doc string says only to
> > bind each binding (bind a binding?!) "as `macrolet2' does".  That
> > means nothing.  Presumably, based on the `*' in the name, the behavior
> > is similar to that of `let*'.  If so, you can use the doc of `let*
> > as inspiration.
>
> Uhm...  I have no idea what it does:
>
> (defmacro macroexp-let2* (test bindings &rest body)
>   "Bind each binding in BINDINGS as `macroexp-let2' does."
>   (declare (indent 2) (debug (sexp (&rest (sexp form)) body)))
>   (pcase-exhaustive bindings
>     ('nil (macroexp-progn body))
>     (`((,var ,exp) . ,tl)
>      `(macroexp-let2 ,test ,var ,exp
>         (macroexp-let2* ,test ,tl ,@body)))))

You understand what macroexp-let2 does?  It supports one binding (a var
plus an expression, specified as separate arguments).  macroexp-let2*
supports a list of such pairs specified as BINDINGS, similar to let*.

The naming scheme `macroexp-let2' vs. `macroexp-let2*' is not ideal:
first, because `macroexp-let2' doesn't support multiple bindings like
`let', and secondly because, if I look at the use cases in the sources,
most of them just want to establish multiple bindings, but parallel
binding would suffice, so they actually want to non-stared version of
macroexp-let2*, which is not macroexp-let2 - AFAICT it doesn't exist.

Michael.

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>>  (defun macroexp-progn (exps)
>> -  "Return an expression equivalent to \\=`(progn ,@EXPS)."
>> +  "Return EXPS with `progn' prepended.
>> +If EXPS is a single expression, `progn' is not prepended."
>>    (if (cdr exps) `(progn ,@exps) (car exps)))
>
> That's described a bit confusingly: AFAIU EXPS should always be a list
> of expressions, and when it's _a_list_ of only one expression, `progn'
> is not prepended and the expression is returned (and not EXPS as your
> text suggests).  BTW, compared to that description I find the original
> version much better (simpler).

Yeah, the doc string is now misleading.  I've rewritten it now as:

---
Return EXPS (a list of expressions) with `progn' prepended.
If EXPS is a list with a single expression, `progn' is not
prepended, but that expression is returned instead.
---

>> Uhm...  I have no idea what it does:
>>
>> (defmacro macroexp-let2* (test bindings &rest body)
>>   "Bind each binding in BINDINGS as `macroexp-let2' does."
>>   (declare (indent 2) (debug (sexp (&rest (sexp form)) body)))
>>   (pcase-exhaustive bindings
>>     ('nil (macroexp-progn body))
>>     (`((,var ,exp) . ,tl)
>>      `(macroexp-let2 ,test ,var ,exp
>>         (macroexp-let2* ,test ,tl ,@body)))))
>
> You understand what macroexp-let2 does?

No, not really.

> It supports one binding (a var plus an expression, specified as
> separate arguments).  macroexp-let2* supports a list of such pairs
> specified as BINDINGS, similar to let*.
>
> The naming scheme `macroexp-let2' vs. `macroexp-let2*' is not ideal:
> first, because `macroexp-let2' doesn't support multiple bindings like
> `let', and secondly because, if I look at the use cases in the sources,
> most of them just want to establish multiple bindings, but parallel
> binding would suffice, so they actually want to non-stared version of
> macroexp-let2*, which is not macroexp-let2 - AFAICT it doesn't exist.

Yes, it's confusing.  But perhaps you could propose a doc string for
`macroexp-let2*'?  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yes, it's confusing.  But perhaps you could propose a doc string for
> `macroexp-let2*'?  :-)

Hmm, it's really just a variant of m-let2 supporting any number of
bindings instead of exactly one.  So, something like

  Multiple binding version of `macroexp-let2'.

  BINDINGS is a list of elements of the form (SYM EXP).  Each EXP can
  refer to symbols specified earlier in the binding list.

maybe?  I'm not good in writing docstrings.

Michael.

bug#19371: 25.0.50; doc of functions and macros defined in macroexp.el

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hmm, it's really just a variant of m-let2 supporting any number of
> bindings instead of exactly one.  So, something like
>
>   Multiple binding version of `macroexp-let2'.
>
>   BINDINGS is a list of elements of the form (SYM EXP).  Each EXP can
>   refer to symbols specified earlier in the binding list.
>
> maybe?  I'm not good in writing docstrings.

Sounds good to me, so I added it to the trunk.  And that was the final
macro mentioned in this bug report, so I'm now closing it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no