Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

Thanks, Daniel.  A few comments about this new facility.

>   cl-lib defstruct introspection

This is rather short: The commit message should be a copy of the
ChangeLog text.

> +The @code{cl-defstruct} package also provides a few structure
> +introspection functions.

Curious: when/where did you bump into this need?

> +@code{setf} place.  @code{cl-struct-slot-value} uses
> +@code{cl-struct-slot-offset} internally and can signal the same
> +errors.

Don't say what it does internally: just mention that it signals the same
errors as cl-struct-slot-offset.  Same for cl-struct-set-slot-value
(even more so there, since you copy&pasted the text without updating 
cl-struct-slot-value to cl-struct-set-slot-value).

> -					 `'(nil ,(cl--const-expr-val def))
> +					 `'(nil ,(cl--const-expr-val
> +                                                  def macroexpand-all-environment))

Please stay within 80 columns.

> +(defmacro cl-the (type form)
> +  "Return FORM.  If type-checking is enabled, assert that it is of TYPE."
>    (declare (indent 1) (debug (cl-type-spec form)))
> +  (if (not (or (not (cl--compiling-file))

Do we really need this cl--compiling-file here?  If it's not absolutely
indispensible, I'd rather avoid using it, since its implementation is
a hideous unreliable hack.

this pu> +(defun cl-struct-sequence-type (struct-type)
> +  "Return the sequence used to build STRUCT-TYPE.
> +STRUCT-TYPE is a symbol naming a struct type.  Return 'vector or
> +'list, or nil if STRUCT-TYPE is not a struct type. "
> +  (car (get struct-type 'cl-struct-type)))
> +(put 'cl-struct-sequence-type 'side-effect-free t)

Could you add a `side-effect-free' thingy to defun-declarations-alist and move
these `put' into a declare?

> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)

Please don't use cl.el facilities here.  Use (declare (gv-setter ...))
instead.  Also, the setter should be improved such that
(incf (cl-struct-slot-value ...)) only computes the offset once.

> +(cl-define-compiler-macro cl-struct-slot-value
> +(cl-define-compiler-macro cl-struct-set-slot-value

These should use the (declare (compiler-macro ...)) facility.
This said, I wonder why they're needed (gets us back to my earlier
question about when you bumped into a need for these facilities).

> +    (&whole orig struct-type slot-name inst)
> +  (or (let* ((macenv macroexpand-all-environment)
> +             (struct-type (cl--const-expr-val struct-type macenv))
> +             (slot-name (cl--const-expr-val slot-name macenv)))
> +        (and struct-type (symbolp struct-type)
> +             slot-name (symbolp slot-name)
> +             (assq slot-name (cl-struct-slot-info struct-type))
> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
> +               (cl-ecase (cl-struct-sequence-type struct-type)
> +                 (vector `(aref (cl-the ,struct-type ,inst) ,idx))
> +                 (list `(nth ,idx (cl-the ,struct-type ,inst)))))))
> +      orig))
[...]
> +    (&whole orig struct-type slot-name inst value)
> +  (or (let* ((macenv macroexpand-all-environment)
> +             (struct-type (cl--const-expr-val struct-type macenv))
> +             (slot-name (cl--const-expr-val slot-name macenv)))
> +        (and struct-type (symbolp struct-type)
> +             slot-name (symbolp slot-name)
> +             (assq slot-name (cl-struct-slot-info struct-type))
> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
> +               (cl-ecase (cl-struct-sequence-type struct-type)
> +                 (vector `(setf (aref (cl-the ,struct-type ,inst) ,idx)
> +                                ,value))
> +                 (list `(setf (nth ,idx (cl-the ,struct-type ,inst))
> +                              ,value))))))
> +      orig))

Try to share code between these two.


        Stefan

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

[ I wrote this yesterday already, but somehow it got eaten by Murphy or
  something.  ]

>   cl-lib defstruct introspection

Please always use the ChangeLog entry as commit message.

> +The @code{cl-defstruct} package also provides a few structure
> +introspection functions.

I'm curious: when/where did you bump against a need for that?

> +@defun cl-struct-set-slot-value struct-type slot-name inst value

We don't need this, since we can always use setf instead.

> -(defun cl--const-expr-val (x)
> +(defun cl--const-expr-val (x &optional environment default)

`environment' is always macroexpand-all-environment, and `default' is
never used, so you can revert this part of the change.

> -					 `'(nil ,(cl--const-expr-val def))
> +					 `'(nil ,(cl--const-expr-val
> +                                                  def macroexpand-all-environment))

Don't go past the 80th column, please.  Undoing the cl--const-expr-val
signature change will fix this problem in most of your patch.

> +(defmacro cl-the (type form)
> +  "Return FORM.  If type-checking is enabled, assert that it is of TYPE."
>    (declare (indent 1) (debug (cl-type-spec form)))
> -  form)
> +  (if (not (or (not (cl--compiling-file))

Unless absolutely needed, we should drop this (cl--compiling-file) test,
because cl--compiling-file is an ugly hack.

> +(defun cl-struct-sequence-type (struct-type)
> +  "Return the sequence used to build STRUCT-TYPE.
> +STRUCT-TYPE is a symbol naming a struct type.  Return 'vector or
> +'list, or nil if STRUCT-TYPE is not a struct type. "
> +  (car (get struct-type 'cl-struct-type)))
> +(put 'cl-struct-sequence-type 'side-effect-free t)

Please add `side-effect-free' to defun-declarations-alist, so we can
move these into `declare's, which is cleaner (it associates them with
the function itself rather than with the shared symbol).

> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)

Please use (declare (gv-setter ...)) or (declare (gv-expand ...)).
Also, we should define it better such that (incf (cl-struct-slot-value ..))
only computes the offset and tests the type once.

> +(cl-define-compiler-macro cl-struct-slot-value

Please use (declare (compiler-macro ..)).

> +    (&whole orig struct-type slot-name inst)
> +  (or (let* ((macenv macroexpand-all-environment)
> +             (struct-type (cl--const-expr-val struct-type macenv))
> +             (slot-name (cl--const-expr-val slot-name macenv)))
> +        (and struct-type (symbolp struct-type)
> +             slot-name (symbolp slot-name)
> +             (assq slot-name (cl-struct-slot-info struct-type))
> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
> +               (cl-ecase (cl-struct-sequence-type struct-type)
> +                 (vector `(aref (cl-the ,struct-type ,inst) ,idx))
> +                 (list `(nth ,idx (cl-the ,struct-type ,inst)))))))
> +      orig))

How important is this optimization?  I mean, if struct-type and
slot-name are constants, then presumably, the code could have used the
accessor function instead, no?
I guess this goes back to the earlier question about when/where the use
for this functionality came up.


        Stefan

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 4173 bytes --]

On 04/21/2014 08:38 AM, Stefan Monnier wrote:
> [ I wrote this yesterday already, but somehow it got eaten by Murphy or
>   something.  ]
> 
>>   cl-lib defstruct introspection
> 
> Please always use the ChangeLog entry as commit message.

That's new. Using the whole ChaneLog message has been a recommendation,
but never a requirement. Now there's one more step on the commit path,
and a useless one at that: the changelog entry is available in the
change itself and in the message to the mailing list.

>> +The @code{cl-defstruct} package also provides a few structure
>> +introspection functions.
> 
> I'm curious: when/where did you bump against a need for that?

I have a few private macros that lexically bind structure slots, and
this information is also needed for some interface-generation work I'm
thinking of doing.

>> +@defun cl-struct-set-slot-value struct-type slot-name inst value
> 
> We don't need this, since we can always use setf instead.

So? We have both (setf (aref ...) ...) and (aset ...).

>> -(defun cl--const-expr-val (x)
>> +(defun cl--const-expr-val (x &optional environment default)
> 
> `environment' is always macroexpand-all-environment, and `default' is
> never used, so you can revert this part of the change.

Sure.

>> +(defmacro cl-the (type form)
>> +  "Return FORM.  If type-checking is enabled, assert that it is of TYPE."
>>    (declare (indent 1) (debug (cl-type-spec form)))
>> -  form)
>> +  (if (not (or (not (cl--compiling-file))
> 
> Unless absolutely needed, we should drop this (cl--compiling-file) test,
> because cl--compiling-file is an ugly hack.

That test was there in cl-check-type. The test doesn't make sense to me
either. We should drop it in both places if we drop it in cl-the.

>> +(defun cl-struct-sequence-type (struct-type)
>> +  "Return the sequence used to build STRUCT-TYPE.
>> +STRUCT-TYPE is a symbol naming a struct type.  Return 'vector or
>> +'list, or nil if STRUCT-TYPE is not a struct type. "
>> +  (car (get struct-type 'cl-struct-type)))
>> +(put 'cl-struct-sequence-type 'side-effect-free t)
> 
> Please add `side-effect-free' to defun-declarations-alist, so we can
> move these into `declare's, which is cleaner (it associates them with
> the function itself rather than with the shared symbol).

Sure.

>> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)
> 
> Please use (declare (gv-setter ...)) or (declare (gv-expand ...)).
> Also, we should define it better such that (incf (cl-struct-slot-value ..))
> only computes the offset and tests the type once.

I've already changed it to gv-define-simple-setter. I don't think the
incf optimization you mention matters.

>> +(cl-define-compiler-macro cl-struct-slot-value
> 
> Please use (declare (compiler-macro ..)).

Why? In both cases, the compiler macro is written out-of-line and in
both cases, we just stick the compiler macro on the symbol's plist.

>> +    (&whole orig struct-type slot-name inst)
>> +  (or (let* ((macenv macroexpand-all-environment)
>> +             (struct-type (cl--const-expr-val struct-type macenv))
>> +             (slot-name (cl--const-expr-val slot-name macenv)))
>> +        (and struct-type (symbolp struct-type)
>> +             slot-name (symbolp slot-name)
>> +             (assq slot-name (cl-struct-slot-info struct-type))
>> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
>> +               (cl-ecase (cl-struct-sequence-type struct-type)
>> +                 (vector `(aref (cl-the ,struct-type ,inst) ,idx))
>> +                 (list `(nth ,idx (cl-the ,struct-type ,inst)))))))
>> +      orig))
> 
> How important is this optimization?  I mean, if struct-type and
> slot-name are constants, then presumably, the code could have used the
> accessor function instead, no?
> I guess this goes back to the earlier question about when/where the use
> for this functionality came up.

Unless we're using this functionality in generated code where, while the
slot is constant, it's more convenient to use that slot's name than to
try to determine the accessor name.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

> That's new. Using the whole ChaneLog message has been a recommendation,
> but never a requirement.

For `elpa', that's true, but for `emacs' it's always been a requirement,
on the premise that this should/will allow us to drop the ChangeLog
files at some point.

> Now there's one more step on the commit path, and a useless one at
> that: the changelog entry is available in the change itself and in the
> message to the mailing list.

C-x v v can copy the message from ChangeLog for you (and set Author:
and Fixes: at the same time), so it's not so bad.

The way to fix this, is to make ChangeLog unneeded.  First step on this
path is to provide some way to make `C-x 4 a' usable without ChangeLog.

>>> +The @code{cl-defstruct} package also provides a few structure
>>> +introspection functions.
>> I'm curious: when/where did you bump against a need for that?
> I have a few private macros that lexically bind structure slots,

Which part makes it impossible/impractical to use standard accessors for
that?

> and this information is also needed for some interface-generation work
> I'm thinking of doing.

Not sure what "interface-generation" means, but it sounds interesting.

>>> +@defun cl-struct-set-slot-value struct-type slot-name inst value
>> We don't need this, since we can always use setf instead.
> So? We have both (setf (aref ...) ...) and (aset ...).

That's only because (setf (aref ...) ...) needs to macroexpand to something.
[ It's one of the differences between Common-Lisp and Elisp.  ]

In your case, (setf (cl-struct-slot-value ...) ...) can macroexpand to
something without needing cl-struct-set-slot-value.  Actually, in order
for (incf (cl-struct-slot-value ...)) not to compute the offset twice,
(setf (cl-struct-slot-value ...) ...) will end up expanding to something
else than a call to cl-struct-set-slot-value.

> That test was there in cl-check-type. The test doesn't make sense to me
> either. We should drop it in both places if we drop it in cl-the.

Great, let's drop it then.  Thanks.

>>> +(cl-define-compiler-macro cl-struct-slot-value
>> Please use (declare (compiler-macro ..)).
> Why? In both cases, the compiler macro is written out-of-line and in
> both cases, we just stick the compiler macro on the symbol's plist.

Because that's the style we use in Elisp.
Note that (declare (compiler-macro ..)) can provide the compiler-macro
"inline" or "out-of-line".

>> I guess this goes back to the earlier question about when/where the use
>> for this functionality came up.
> Unless we're using this functionality in generated code where, while the
> slot is constant, it's more convenient to use that slot's name than to
> try to determine the accessor name.

Ah, so it's for code generated based on cl-struct-slot-info?
Right, that makes sense.


        Stefan

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 6891 bytes --]

On 04/21/2014 03:03 PM, Stefan Monnier wrote:
>> That's new. Using the whole ChaneLog message has been a recommendation,
>> but never a requirement.
> 
> For `elpa', that's true, but for `emacs' it's always been a requirement,
> on the premise that this should/will allow us to drop the ChangeLog
> files at some point.

Thanks for the clarification.

>> Now there's one more step on the commit path, and a useless one at
>> that: the changelog entry is available in the change itself and in the
>> message to the mailing list.
> 
> C-x v v can copy the message from ChangeLog for you (and set Author:
> and Fixes: at the same time), so it's not so bad.

Can it? I tried it in vc-dir and got a completely unrelated ChangeLog hunk.

> The way to fix this, is to make ChangeLog unneeded.  First step on this
> path is to provide some way to make `C-x 4 a' usable without ChangeLog.
> 
>>>> +The @code{cl-defstruct} package also provides a few structure
>>>> +introspection functions.
>>> I'm curious: when/where did you bump against a need for that?
>> I have a few private macros that lexically bind structure slots,
> 
> Which part makes it impossible/impractical to use standard accessors for
> that?

:conc-name, for starters. Also, :read-only, although you could argue
that you shouldn't go around mutating read-only slots anyway.

>> and this information is also needed for some interface-generation work
>> I'm thinking of doing.
> 
> Not sure what "interface-generation" means, but it sounds interesting.

I'm just playing around at this point --- but the basic idea is that we
need some way to connect independent Emacs components, and this
mechanism should be more structured than some kind of
do-everything-in-a-giant-cond handler function, but much lighter and
more comprehensible than EIEIO.

One approach is to define COM-like (wait! keep reading!) "interface"
structures that bundle useful functions into composeable pieces, then
have Emacs components interact in terms of these interfaces. We can make
them very lightweight, and let all interfaces inherit from a common
interface that lets callers ask for additional functionality:

(iface-declare (iface-base nil)
  "Interface from which all others inherit."
  ((iface-query instance interface)
   "Get an implementation of INTERFACE from INSTANCE."))

For example, we could define some project interfaces like this:

(iface-declare (project iface-base)
  ((project-get-root-directory project )
   "Find the root directory of PROJECT.")
  ((project-get project property)
   "Get a project property PROPERTY.")
  ((project-put project property value)
   "Set a project property PROPERTY to VALUE."))

(iface-declare (project-c iface-base)
  ((project-c-include-directories project file)
   "Return a list of include directories for FILE in PROJECT.
FILE is a fully-qualified name."))

iface-declare defines trivial wrapper functions that make it convenient
to call through interface fields. So the generated definition of
project-get, for example, would look like this:

(defun project-get (inst &rest xargs)
    "Get a project property PROPERTY."
    (declare (advertised-calling-convention (project property))
    (apply (cl-struct-slot-value 'project 'get inst) xargs))

If you have a project and want to see whether it supports finding C
include paths, you'd just use iface-query with 'project-c, and if that
succeeds, call (project-c-include-directories my-c-project file).

You'd actually *make* interface instances by building an interface
struct as you would any other struct and supply closures (e.g., the
result of `apply-partially') for the slot values so that you can
maintain state (or not) between subsequent calls on the same interface
instance.

Anyway, all of this would be very lightweight and (I think) resolvable
completely at compile time.

Note that it's probably not a good idea to go even simpler and have,
say, my-git-project inherit from project directly: what if the size of
project changes? (Things would break because cl-defstruct accessors
hardcode field offsets, and that's a good thing.) What if you want to
implement multiple pieces of optional functionality? (Sure, you can make
some struct fields nullable, but then you have to provide some way to
query whether a field is actually implemented.)

Better to just define additional interfaces for optional functionality.
The interface approach seems decently light and extensible, and the
implementation complexity is low.

>>>> +@defun cl-struct-set-slot-value struct-type slot-name inst value
>>> We don't need this, since we can always use setf instead.
>> So? We have both (setf (aref ...) ...) and (aset ...).
> 
> That's only because (setf (aref ...) ...) needs to macroexpand to something.
> [ It's one of the differences between Common-Lisp and Elisp.  ]
> 
> In your case, (setf (cl-struct-slot-value ...) ...) can macroexpand to
> something without needing cl-struct-set-slot-value.  Actually, in order
> for (incf (cl-struct-slot-value ...)) not to compute the offset twice,
> (setf (cl-struct-slot-value ...) ...) will end up expanding to something
> else than a call to cl-struct-set-slot-value.
> 
>> That test was there in cl-check-type. The test doesn't make sense to me
>> either. We should drop it in both places if we drop it in cl-the.
> 
> Great, let's drop it then.  Thanks.

Will do. That test never made much sense to me anyway.

>>>> +(cl-define-compiler-macro cl-struct-slot-value
>>> Please use (declare (compiler-macro ..)).
>> Why? In both cases, the compiler macro is written out-of-line and in
>> both cases, we just stick the compiler macro on the symbol's plist.
> 
> Because that's the style we use in Elisp.

Well, recently. ;-) It wasn't too long ago that the style was to
let-bind dozens of variables at the top of huge functions and setq them
everywhere.

> Note that (declare (compiler-macro ..)) can provide the compiler-macro
> "inline" or "out-of-line".

Sure, but it'll be really ugly if it's inline and relatively large (as
the macros here are). It goes with a trend I've noticed in the APIs
you've implemented, though: you seem to prefer having people defun
regular functions and combine them in interesting ways (e.g., with
advice-add) instead of using top-level definition forms directly (e.g.,
defadvice). I largely agree with your approach.

> 
>>> I guess this goes back to the earlier question about when/where the use
>>> for this functionality came up.
>> Unless we're using this functionality in generated code where, while the
>> slot is constant, it's more convenient to use that slot's name than to
>> try to determine the accessor name.
> 
> Ah, so it's for code generated based on cl-struct-slot-info?
> Right, that makes sense.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

> Can it? I tried it in vc-dir and got a completely unrelated ChangeLog hunk.

Yes, there's a long-standing problem in vc-dir, such that it only works
well if you mark the files you're going to commit (basically the
changelog-copying feature needs the list of files that will be committed
and vc-dir doesn't provide it unless you manually mark each file).

Patch welcome.

>>>> I'm curious: when/where did you bump against a need for that?
>>> I have a few private macros that lexically bind structure slots,
>> Which part makes it impossible/impractical to use standard accessors for
>> that?
> :conc-name, for starters. Also, :read-only, although you could argue
> that you shouldn't go around mutating read-only slots anyway.

I guess I don't know what you mean by "lexically bind structure slots".
Can you give an example?

> Better to just define additional interfaces for optional functionality.
> The interface approach seems decently light and extensible, and the
> implementation complexity is low.

An object system based on interfaces sounds fine (tho I'm not sure if
it'll really end up simpler).  But note that every closure is made of
2 arrays, so a struct of 4 closures will really be made of 9 arrays, and
usually the only thing closed-over will be "the additional data"
(i.e. the attributes usually stored in "self"), so if we could handle
this object (the "self") specially we'd avoid all 4 closures.


        Stefan

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

On 04/21/2014 07:03 PM, Stefan Monnier wrote:
>> Can it? I tried it in vc-dir and got a completely unrelated ChangeLog hunk.
> 
> Yes, there's a long-standing problem in vc-dir, such that it only works
> well if you mark the files you're going to commit (basically the
> changelog-copying feature needs the list of files that will be committed
> and vc-dir doesn't provide it unless you manually mark each file).
> 
> Patch welcome.
> 
>>>>> I'm curious: when/where did you bump against a need for that?
>>>> I have a few private macros that lexically bind structure slots,
>>> Which part makes it impossible/impractical to use standard accessors for
>>> that?
>> :conc-name, for starters. Also, :read-only, although you could argue
>> that you shouldn't go around mutating read-only slots anyway.
> 
> I guess I don't know what you mean by "lexically bind structure slots".
> Can you give an example?

Something like this. (This is the private code I was talking about, so
it doesn't use the sames in the checked-in patch.)

(defmacro* jez-with-slots (spec-list (type inst) &body body)
  "Like `with-slots', but for structs."
  (if (symbolp inst)
      `(symbol-macrolet
           ,(loop for spec in spec-list
                  collect `(,spec (jez-slot-value ',type ,inst ',spec)))
         ,@body)
    (let ((inst-symbol (gensym "with-struct-slots")))
      `(let ((,inst-symbol ,inst))
         (jez-with-slots
             ,spec-list ,inst-symbol ,@body)))))


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

> Something like this. (This is the private code I was talking about, so
> it doesn't use the sames in the checked-in patch.)

> (defmacro* jez-with-slots (spec-list (type inst) &body body)
>   "Like `with-slots', but for structs."
>   (if (symbolp inst)
>       `(symbol-macrolet
>            ,(loop for spec in spec-list
>                   collect `(,spec (jez-slot-value ',type ,inst ',spec)))
>          ,@body)
>     (let ((inst-symbol (gensym "with-struct-slots")))
>       `(let ((,inst-symbol ,inst))
>          (jez-with-slots
>              ,spec-list ,inst-symbol ,@body)))))

Ah, I see now, thanks, yes that makes sense.


        Stefan


PS: Of course, I wouldn't implement it that way ;-)
I'd start by testing `inst' against `type' and then use straight (aref
...)  or (nth ...) for the accessors.
But indeed, you'd want to circumvent the usual accessors.

Declaim and proclaim (Was: Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection)

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On 04/21/2014 03:03 PM, Stefan Monnier wrote:
>> That test was there in cl-check-type. The test doesn't make sense to me
>> either. We should drop it in both places if we drop it in cl-the.
> 
> Great, let's drop it then.  Thanks.

On second thought, I'm not sure it's so simple. First of all, nobody in
the tree actually changes the default speed or safety settings, AFAICT.
On closer inspection, maybe that's been a good thing: cl--optimize-speed
and cl--optimize-safety are ordinary non-local variables, and
cl--do-proclaim just setqs them, indicating that compiling files that
change their optimization settings will have global effects. I'm not
sure we should change how it works now without having a discussion of
how we want this whole system to work.

Should we make cl--optimize-speed and cl--optimize-safety buffer-local?
And shouldn't we be setting the default values to 3 and 0, respectively,
during initial Emacs compilation when we haven't been given configured
with --enable-checking?

Also, we could make cl-locally do something useful by having it bind
cl--optimize-speed and cl--optimize-safety, then fully macroexpand its body.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [Emacs-diffs] trunk r116995: cl-lib defstruct introspection

[Stefan Monnier]

Oh, well, looks like the original answer wasn't completely lost
after all.  It was just stuck in some mail queue!
Make sure you disregard it, tho.  I think we've hashed this enough already.


        Stefan

>>>>> "Stefan" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Thanks, Daniel.  A few comments about this new facility.
>> cl-lib defstruct introspection

> This is rather short: The commit message should be a copy of the
> ChangeLog text.

>> +The @code{cl-defstruct} package also provides a few structure
>> +introspection functions.

> Curious: when/where did you bump into this need?

>> +@code{setf} place.  @code{cl-struct-slot-value} uses
>> +@code{cl-struct-slot-offset} internally and can signal the same
>> +errors.

> Don't say what it does internally: just mention that it signals the same
> errors as cl-struct-slot-offset.  Same for cl-struct-set-slot-value
> (even more so there, since you copy&pasted the text without updating 
> cl-struct-slot-value to cl-struct-set-slot-value).

>> -					 `'(nil ,(cl--const-expr-val def))
>> +					 `'(nil ,(cl--const-expr-val
>> +                                                  def macroexpand-all-environment))

> Please stay within 80 columns.

>> +(defmacro cl-the (type form)
>> +  "Return FORM.  If type-checking is enabled, assert that it is of TYPE."
>> (declare (indent 1) (debug (cl-type-spec form)))
>> +  (if (not (or (not (cl--compiling-file))

> Do we really need this cl--compiling-file here?  If it's not absolutely
> indispensible, I'd rather avoid using it, since its implementation is
> a hideous unreliable hack.

> this pu> +(defun cl-struct-sequence-type (struct-type)
>> +  "Return the sequence used to build STRUCT-TYPE.
>> +STRUCT-TYPE is a symbol naming a struct type.  Return 'vector or
>> +'list, or nil if STRUCT-TYPE is not a struct type. "
>> +  (car (get struct-type 'cl-struct-type)))
>> +(put 'cl-struct-sequence-type 'side-effect-free t)

> Could you add a `side-effect-free' thingy to defun-declarations-alist and move
> these `put' into a declare?

>> +(defsetf cl-struct-slot-value cl-struct-set-slot-value)

> Please don't use cl.el facilities here.  Use (declare (gv-setter ...))
> instead.  Also, the setter should be improved such that
> (incf (cl-struct-slot-value ...)) only computes the offset once.

>> +(cl-define-compiler-macro cl-struct-slot-value
>> +(cl-define-compiler-macro cl-struct-set-slot-value

> These should use the (declare (compiler-macro ...)) facility.
> This said, I wonder why they're needed (gets us back to my earlier
> question about when you bumped into a need for these facilities).

>> +    (&whole orig struct-type slot-name inst)
>> +  (or (let* ((macenv macroexpand-all-environment)
>> +             (struct-type (cl--const-expr-val struct-type macenv))
>> +             (slot-name (cl--const-expr-val slot-name macenv)))
>> +        (and struct-type (symbolp struct-type)
>> +             slot-name (symbolp slot-name)
>> +             (assq slot-name (cl-struct-slot-info struct-type))
>> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
>> +               (cl-ecase (cl-struct-sequence-type struct-type)
>> +                 (vector `(aref (cl-the ,struct-type ,inst) ,idx))
>> +                 (list `(nth ,idx (cl-the ,struct-type ,inst)))))))
>> +      orig))
> [...]
>> +    (&whole orig struct-type slot-name inst value)
>> +  (or (let* ((macenv macroexpand-all-environment)
>> +             (struct-type (cl--const-expr-val struct-type macenv))
>> +             (slot-name (cl--const-expr-val slot-name macenv)))
>> +        (and struct-type (symbolp struct-type)
>> +             slot-name (symbolp slot-name)
>> +             (assq slot-name (cl-struct-slot-info struct-type))
>> +             (let ((idx (cl-struct-slot-offset struct-type slot-name)))
>> +               (cl-ecase (cl-struct-sequence-type struct-type)
>> +                 (vector `(setf (aref (cl-the ,struct-type ,inst) ,idx)
>> +                                ,value))
>> +                 (list `(setf (nth ,idx (cl-the ,struct-type ,inst))
>> +                              ,value))))))
>> +      orig))

> Try to share code between these two.


>         Stefan

Re: Declaim and proclaim

[Stefan Monnier]

>>> That test was there in cl-check-type. The test doesn't make sense to me
>>> either. We should drop it in both places if we drop it in cl-the.
>> Great, let's drop it then.  Thanks.
> On second thought, I'm not sure it's so simple. First of all, nobody in
> the tree actually changes the default speed or safety settings, AFAICT.

Could you explain what this discussion has to do with the
removal of the cl--compiling-file calls?

> sure we should change how it works now without having a discussion of
> how we want this whole system to work.

It's not terribly important, tho.  Given the simplicity of the
implementation techniques we use, this CL optimization infrastructure is
over-engineered.

> Should we make cl--optimize-speed and cl--optimize-safety buffer-local?
> And shouldn't we be setting the default values to 3 and 0, respectively,
> during initial Emacs compilation when we haven't been given configured
> with --enable-checking?

What would that change?

> Also, we could make cl-locally do something useful by having it bind
> cl--optimize-speed and cl--optimize-safety, then fully macroexpand its body.

Not worth the trouble, methinks.


        Stefan