bug#7070: `load-time-value' local-vars this-kind and that-one

bug#7070: `load-time-value' local-vars this-kind and that-one

[MON KEY]

current through BZR-101480 lisp/cl-macs.el macro `load-time-value' has this
branch which if unsatisfied is supposed to fset `byte-compile-file-form':

 (and (fboundp 'byte-compile-file-form-defmumble)
      (boundp 'this-kind) (boundp 'that-one))

However, luckily the above doesn't happen.

The only locations that bind the variables like this are in
byte-compile-file-form-defmumble' e.g. the local letbound vars
bytecomp-this-kind' and `bytecomp-that-one'.

Until fairly recently (revision 94414) the symbols `this-kind' and and
`that-one' were present in the bytecomp.el

There is a comment about this which says:
,----
| ;; The `bytecomp-' prefix is applied to all local variables with
| ;; otherwise common names in this and similar functions for the sake
| ;; of the boundp test in byte-compile-variable-ref.
| ;; http://lists.gnu.org/archive/html/emacs-devel/2008-01/msg00237.html
| ;; http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-02/msg00134.html
| ;; Note that similar considerations apply to command-line-1 in startup.el.
`----

The symbols (this-kind and that-one) appear to have been changed here:

,----
| ------------------------------------------------------------
| revno: 96701
| committer: Glenn Morris <rgm@gnu.org>
| timestamp: Thu 2009-07-23 02:53:29 +0000
| message:
|   (byte-compile-from-buffer, byte-compile-output-file-form)
|   (byte-compile-output-docform, byte-compile-file-form-defmumble)
|   (byte-compile-output-as-comment):
|   Give some more local variables with common names a "bytecomp-" prefix,
|   so as not to shadow things during compilation.
| ------------------------------------------------------------
|
| ------------------------------------------------------------
| revno: 96702
| committer: Glenn Morris <rgm@gnu.org>
| timestamp: Thu 2009-07-23 02:54:00 +0000
| message:
|   (load-time-value): Update for the name-change `outbuffer' to
|   `bytecomp-outbuffer'.
| ------------------------------------------------------------
|
`----

Revision 96701 renamed the `byte-compile-file-form-defmumble's local vars:

`this-kind' -> `bytecomp-this-kind'
`this-one'  -> `bytecomp-this-one'
`that-one'  -> `bytecomp-that-one'
`outbuffer' -> `bytecomp-outbuffer'

Revision 96702 renamed the `load-time-value' local var:

`outbuffer' -> `bytecomp-outbuffer'

but it appears to have left the `this-one' and `this-kind' vars unchanged.

Unless I am misunderstanding/missing something either:

a) some special magic that happens invisibily when Emacs is dumped and the
   current constraint is some sort of backward compatibility kludge;

b) the macro never worked correctly;

c) revisions at 96701/96702 created a bug where `load-time-value's can no longer
   alter via fset the function cell of `byte-compile-file-form' because the
   following constraint can not be satisfied;

d) the macro never worked correctly and revisions at 96701/96702 created an
   additional bug;

My money is on d.

Right now if the macro is change to have the following form instead:

 (and (fboundp 'byte-compile-file-form-defmumble)
      (boundp 'bytecomp-this-kind) (boundp 'bytecomp-that-one))

`byte-compile-file-form' _can_ get rebound and this is not good.  Moreover, even
with this (further breaking) change I am not able to obtain the behavior
described in the manual at: (info "(cl)Time of Evaluation")

Is there some reason why the entire inner branch can't be elided with the macro
written as:

,---- (lightly tested)
|
| (defmacro load-time-value (form &optional read-only)
|   "Like `progn', but evaluates the body at load time.
|  The result of the body appears to the compiler as a quoted constant."
|   (if (cl-compiling-file)
|       (print `(set ',(make-symbol "--cl-load-time--") ,@form)
|              (symbol-value 'bytecomp-outbuffer))
|     `(eval ',form)))
|
`----

Also, what does the READ-ONLY arg do?

The only possibility I can think of is that it somehow dovetails with the
:read-only key in defstruct.

--
/s_P\

bug#7070: `load-time-value' local-vars this-kind and that-one

[npostavs]

severity 7070 minor
quit

MON KEY <monkey@sandpframing.com> writes:

> current through BZR-101480 lisp/cl-macs.el macro `load-time-value' has this
> branch which if unsatisfied is supposed to fset `byte-compile-file-form':
>
>  (and (fboundp 'byte-compile-file-form-defmumble)
>       (boundp 'this-kind) (boundp 'that-one))
>
> However, luckily the above doesn't happen.
>
> The only locations that bind the variables like this are in
> byte-compile-file-form-defmumble' e.g. the local letbound vars
> bytecomp-this-kind' and `bytecomp-that-one'.

[...]

>
> Revision 96701 renamed the `byte-compile-file-form-defmumble's local vars:
>
> `this-kind' -> `bytecomp-this-kind'
> `this-one'  -> `bytecomp-this-one'
> `that-one'  -> `bytecomp-that-one'
> `outbuffer' -> `bytecomp-outbuffer'
>
> Revision 96702 renamed the `load-time-value' local var:
>
> `outbuffer' -> `bytecomp-outbuffer'
>
> but it appears to have left the `this-one' and `this-kind' vars unchanged.

Meanwhile, bytecomp.el has switched to lexical binding, so the names of
this-kind and that-one are irrelevant; they will never be bound.

>
> Unless I am misunderstanding/missing something either:
>
> a) some special magic that happens invisibily when Emacs is dumped and the
>    current constraint is some sort of backward compatibility kludge;
>
> b) the macro never worked correctly;
>
> c) revisions at 96701/96702 created a bug where `load-time-value's can no longer
>    alter via fset the function cell of `byte-compile-file-form' because the
>    following constraint can not be satisfied;
>
> d) the macro never worked correctly and revisions at 96701/96702 created an
>    additional bug;
>
> My money is on d.

I'm not sure about past behaviour, but with current Emacs, the macro
does appear to work correctly.  AFAICT, it's because the condition the
boundp calls are trying to protect against (that the compiler is in the
middle of printing out a function's code) never occurs because the
compiler prints the code of a function in a single shot (perhaps because
it has to run optimizations on the whole function before printing?).  In
other words this macro could be simplified to

    (defmacro cl-load-time-value (form &optional _read-only)
      "Like `progn', but evaluates the body at load time.
    The result of the body appears to the compiler as a quoted constant."
      (declare (debug (form &optional sexp)))
      (if (cl--compiling-file)
          (let* ((temp (cl-gentemp "--cl-load-time--")))
            ;; Assume there are no unclosed parens in
            ;; `byte-compile--outbuffer'.
            (print `(setq ,temp ,form) byte-compile--outbuffer)
            temp)
        `',(eval form)))

And it should probably use `defconst' instead of `setq' to avoid a free
variable warning.

>
> Also, what does the READ-ONLY arg do?
>
> The only possibility I can think of is that it somehow dovetails with the
> :read-only key in defstruct.

It does nothing in Emacs' implementation, perhaps it could use
`purecopy'?  Although that would still do nothing most of the time, so
maybe it's not worth the trouble.

According to cltl (https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node224.html)

  The optional read-only-p argument designates whether the result may
  be considered a read-only constant. If nil (the default), the result
  must be considered ordinary, modifiable data. If t, the result is a
  read-only quantity that may, as appropriate, be copied into
  read-only space and may, as appropriate, be shared with other
  programs. The read-only-p argument is not evaluated and only the
  literal symbols t and nil are permitted.

bug#7070: `load-time-value' local-vars this-kind and that-one

[Lars Ingebrigtsen]

npostavs@users.sourceforge.net writes:

> I'm not sure about past behaviour, but with current Emacs, the macro
> does appear to work correctly. 

cl-load-time-value has changed even more since this, so I'm not sure
there's anything more to fix here?  (Hm...  but the last major rewrite
was in 2015...)

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

bug#7070: `load-time-value' local-vars this-kind and that-one

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> npostavs@users.sourceforge.net writes:
>
>> I'm not sure about past behaviour, but with current Emacs, the macro
>> does appear to work correctly. 
>
> cl-load-time-value has changed even more since this, so I'm not sure
> there's anything more to fix here?  (Hm...  but the last major rewrite
> was in 2015...)

More information was requested, but no response was given within a
month, so I'm closing this bug report.  If the problem still exists,
please respond to this email and we'll reopen the bug report.

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