GNU bug report logs - #77058
[PATCH] Add cl-with-accessors

Previous Next

Package: emacs;

Reported by: Okamsn <okamsn <at> protonmail.com>

Date: Sun, 16 Mar 2025 18:25:02 UTC

Severity: normal

Tags: patch

Done: Stefan Monnier <monnier <at> iro.umontreal.ca>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: okamsn <at> protonmail.com
To: Daniel Colascione <dancol <at> dancol.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77058 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: bug#77058: [PATCH] Add cl-with-accessors
Date: Sat, 29 Mar 2025 23:56:48 +0000
[Message part 1 (text/plain, inline)]
Hello,

I apologize for missing Stefan's response.

Daniel Colascione wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>> Ping! Okamsn, would you like to submit an updated patch?
>>
 >>> Stefan Monnier wrote:>>>> Also, in the test 
`cl-lib-struct-accessors' in
>>>> test/lisp/emacs-lisp/cl-lib-tests.el, it checks for the keyword
>>>> `:readonly', but the documented keyword is `:read-only'.  Is this a
>>>> typo, or was this intentional?
>>>
>>> I'd go with "typo".

I've added a separate patch file to change it from `:readonly' to 
`:read-only'.

>>> It'd be worthwhile to mention that it also works with accessors of other
>>> objects than structs, e.g. `car`, or `process-filter`.
 >>> ...
>>> so maybe it's worth mentioning `with-slots` (which arguably should be
>>> moved to `cl-lib`).

I've mentioned `with-slots' and that `cl-with-accessors' works with `car'.

>>> I think this should be moved to a CL-Lib subsection in the
>>> "package-specific changes" section.

Done. I did not move the entry about `cl-type-of'.  I see that it is 
written in C.

>>> I'd use `VAR` or `SYMBOL` rather than `SYMBOL-NAME`.
>>> Probably similarly use `ACCESSOR` rather than `ACCESSOR-NAME`.
>>> ...
>>> Should be lower case `&rest` on this last line.
 >>> ...
>>> I think `place` is wrong here.  `(person-age FOO)` is a "place" but
>>> `person-age` is not (or, well, it is but not the right one: when taken
>>> as a place it refers to the variable, not the accessor).
>>> I think it should just be `symbolp`.
>>> ...
>>> Please use the same format as other similar warnings we have, such as:
>>>

I've made these changes. I changed SYMBOL-NAME to NAME to be consistent 
with `cl-symbol-macrolet'.

> ...
 > You could get away from> the need for compile-time visible of classes 
by switching from
> cl-symbol-macrolet to a custom expander function (which is how
> cl-symbol-macrolet itself is implemented), but I was too lazy.

Daniel,

I wrote `cl-with-accessors' with `cl-symbol-macrolet' because the 
definition at http://clhs.lisp.se/Body/m_w_acce.htm used 
`symbol-macrolet'. Do you believe that it should be done differently or 
be more like what you wrote?

Thank you.
[v3-0001-Fix-typo-in-test-of-read-only-cl-defstruct-slot.patch (text/x-patch, attachment)]
[v3-0002-Add-cl-with-accessors.patch (text/x-patch, attachment)]

This bug report was last modified 50 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.