Recent change in master breaks async package update using paradox

Recent change in master breaks async package update using paradox

[Kaushal Modi]

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

Hi all,

A recent change in master (last 3-4 days) causes async package updates
using paradox.el to throw this error:

Debugger entered--Lisp error: (wrong-type-argument sequencep
#s(package-desc swiper (20170328 737) "Isearch with an overview. Oh, man!"
((emacs (24 1)) (ivy (0 8 0))) nil nil
"/home/kmodi/.emacs.d/elpa_26/swiper-20170328.737" ((:keywords "matching")
(:url . "https://github.com/abo-abo/swiper") (:stars #("249" 0 3
(font-lock-face paradox-starred-face)))) nil))
  signal(wrong-type-argument (sequencep #s(package-desc swiper (20170328
737) "Isearch with an overview. Oh, man!" ((emacs (24 1)) (ivy (0 8 0)))
nil nil "/home/kmodi/.emacs.d/elpa_26/swiper-20170328.737" ((:keywords
"matching") (:url . "https://github.com/abo-abo/swiper") (:stars #("249" 0
3 (font-lock-face paradox-starred-face)))) nil)))
  async-handle-result((lambda (x) (setq package-alist (pop x)
package-selected-packages (pop x) package-archive-contents (pop x)) (when
(spinner-p paradox--spinner) (spinner-stop paradox--spinner) (setq
paradox--spinner nil)) (setq paradox--executing nil) (run-hook-with-args
(quote paradox-after-execute-functions) (pop x))
(paradox--post-execute-star-unstar (quote nil) (paradox--repo-alist)))
(async-signal (wrong-type-argument sequencep #s(package-desc swiper
(20170328 737) "Isearch with an overview. Oh, man!" ((emacs (24 1)) (ivy (0
8 0))) nil nil "/home/kmodi/.emacs.d/elpa_26/swiper-20170328.737"
((:keywords "matching") (:url . "https://github.com/abo-abo/swiper")
(:stars #("249" 0 3 (font-lock-face paradox-starred-face)))) nil)))
#<buffer *emacs*>)

The backtrace doesn't point to the part of paradox.el that causes this
error so I cannot debug it.

So a wild guess is that one of these is causing this issue:

f1d34d9136 * master origin/master origin/HEAD Minor cleanups related to
type-of
ab0a27517c * ; * lisp/emacs-lisp/package.el (describe-package-1): Tweak
recent.
fe8cf45710 * * doc/lispref/package.texi (Package Archives): Mention https.
f981c611e2 * Advertise https for homepage of gnu.org packages
048133d488 * Default to https for elpa.gnu.org if gnutls available
82b9efc869 * Minor simplifications and doc for records
12317ff4fb * Fix recent changes in record data type
604eb02fff * Make subprocess functions resolve the default directory
f4b50dad8d * Make ediff handle remote and quoted file names
2c68192c6b * Backward compatibility with pre-existing struct instances.
b6738682ae * Make the URL library use records.
8e6f204f44 * Make EIEIO use records.
0565482838 * Make cl-defstruct use records.
a2c3343029 * Add record objects with user-defined types.

Questions:

- Can anyone recreate this issue using async update using paradox using the
latest master?
- How would one debug using that backtrace?

https://github.com/Malabarba/paradox/issues/139
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3739 bytes --]

Re: Recent change in master breaks async package update using paradox

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Wed, 05 Apr 2017 15:31:10 +0000
> 
> A recent change in master (last 3-4 days) causes async package updates using paradox.el to throw this
> error:

How did you rebuild Emacs after fetching this commit:

> a2c3343029 * Add record objects with user-defined types.

Did you recompile _all_ Lisp files?  If not, please do, and then try
again.

Re: Recent change in master breaks async package update using paradox

[Tom Tromey]

>>>>> "Kaushal" == Kaushal Modi <kaushal.modi@gmail.com> writes:

Kaushal> Debugger entered--Lisp error: (wrong-type-argument sequencep #s(package-desc swiper (20170328

Given this, it's probably due to the change to give structures a real
type.

Did you recompile paradox?  I would start by trying that.

Kaushal> 8e6f204f44 * Make EIEIO use records.
Kaushal> 0565482838 * Make cl-defstruct use records.

Most likely one of these two.

Tom

Re: Recent change in master breaks async package update using paradox

[Kaushal Modi]

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

On Wed, Apr 5, 2017 at 5:59 PM Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kaushal" == Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> Kaushal> Debugger entered--Lisp error: (wrong-type-argument sequencep
> #s(package-desc swiper (20170328
>
> Given this, it's probably due to the change to give structures a real
> type.
>
> Did you recompile paradox?  I would start by trying that.
>

I deleted the .elc files, restarted emacs and tried again and still get the
same error (only when updating/installing packages asynchronously).

Kaushal> 8e6f204f44 * Make EIEIO use records.
> Kaushal> 0565482838 * Make cl-defstruct use records.
>
> Most likely one of these two.


Thanks. I'll try reverting those.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1576 bytes --]

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi <kaushal.modi@gmail.com> writes:
> I deleted the .elc files, restarted emacs and tried again and still
> get the same error (only when updating/installing packages
> asynchronously).

You could also try a full "make bootstrap".

Could you please tell me the exact steps you did to get the error?
I'd like to try to reproduce it myself.

Re: Recent change in master breaks async package update using paradox

[Kaushal Modi]

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

On Thu, Apr 6, 2017, 1:00 AM Lars Brinkhoff <lars@nocrew.org> wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
> > I deleted the .elc files, restarted emacs and tried again and still
> > get the same error (only when updating/installing packages
> > asynchronously).
>
> You could also try a full "make bootstrap".
>


I did not confirm but I thought that should have happened because I did
rebuild emacs with `make bootstrap`.

There was no option because before `make bootstrap`, I got:

Loading emacs-lisp/timer...
Loading isearch...
Attempt to autoload cl-old-struct-compat-mode while preparing to dump
make[1]: *** [emacs] Error 255
make[1]: Leaving directory `/home/kmodi/downloads/git/emacs/src'
make: *** [src] Error 2

Could you please tell me the exact steps you did to get the error?
> I'd like to try to reproduce it myself.
>

- Install paradox from GNU Elpa
- (paradox-enable)
- M-x list-packages (This will now open the default package manager, but
with additions made by paradox).
- Install any package ( i x ), and then hit 'y' when asked if you want to
do that asynchronously.
- You will then see the error I mentioned in my first email of this thread.

I will open a bug report for this today so that all this info can be
contained in there.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2318 bytes --]

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi <kaushal.modi@gmail.com> writes:
>     Could you please tell me the exact steps you did to get the error?
>     I'd like to try to reproduce it myself.
>
> - Install paradox from GNU Elpa

I didn't see it in Elpa, so I installed it from Melpa.

> - (paradox-enable)

Had to type (require 'paradox)

> - M-x list-packages (This will now open the default package manager,
> but with additions made by paradox).
> - Install any package ( i x ), and then hit 'y' when asked if you want
> to do that asynchronously. 

Had to install async first.

> - You will then see the error I mentioned in my first email of this
> thread. 

I successfully installed "0xc" asynchronously.

I'll check out the very latest master and do a full rebuilt from a clean
repo, and then redo the steps.

Re: Recent change in master breaks async package update using paradox

[Kaushal Modi]

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

On Thu, Apr 6, 2017, 7:01 AM Lars Brinkhoff <lars@nocrew.org> wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
> >     Could you please tell me the exact steps you did to get the error?
> >     I'd like to try to reproduce it myself.
> >
> > - Install paradox from GNU Elpa
>
> I didn't see it in Elpa, so I installed it from Melpa.
>
> > - (paradox-enable)
>
> Had to type (require 'paradox)
>
> > - M-x list-packages (This will now open the default package manager,
> > but with additions made by paradox).
> > - Install any package ( i x ), and then hit 'y' when asked if you want
> > to do that asynchronously.
>
> Had to install async first.
>
> > - You will then see the error I mentioned in my first email of this
> > thread.
>
> I successfully installed "0xc" asynchronously.
>
> I'll check out the very latest master and do a full rebuilt from a clean
> repo, and then redo the steps.
>

Thanks for attempting to recreate the issue. When I get to a computer, I'll
put in the full details of how to recreate this from an emacs -Q session in
a separate bug thread.

As you mentioned in the paradox GitHub thread, a problem could be that when
async runs on my machine, it probably runs an older Emacs.

Would you or anyone on the list have an idea if we can specify which exact
emacs binary should be used for this "other" emacs process?

I build emacs locally at work, and set the PATH in my shell config. Without
that, the default emacs version is 23.x. I would have expected async to
spawn off another emacs instance of the same version/binary as the parent
process. Is that not the case?

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2891 bytes --]

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi wrote:
> As you mentioned in the paradox GitHub thread, a problem could be that
> when async runs on my machine, it probably runs an older Emacs. 
>
> Would you or anyone on the list have an idea if we can specify which
> exact emacs binary should be used for this "other" emacs process?

According to this, it looks like it just starts the first "emacs"
found in the path.

https://github.com/jwiegley/emacs-async/blob/master/async.el#L287

But then, when I tested this, I didn't change PATH so I think async
should have run my /usr/bin/emacs, which is 24.3.

Another theory is that some .elc in your local ~/.emacs.d/elpa would
have to be recompiled.  But that's a use case we should support through
(cl-old-struct-mode 1), so I'd like to fix that.

Re: Recent change in master breaks async package update using paradox

[Noam Postavsky]

On Thu, Apr 6, 2017 at 8:12 AM, Lars Brinkhoff <lars@nocrew.org> wrote:
>
> According to this, it looks like it just starts the first "emacs"
> found in the path.
>
> https://github.com/jwiegley/emacs-async/blob/master/async.el#L287

No, this code should find the executable of the currently running emacs:

    (file-truename
     (expand-file-name invocation-name
                       invocation-directory))

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Noam Postavsky <npostavs@users.sourceforge.net> writes:
> No, this code should find the executable of the currently running emacs:
>
>     (file-truename
>      (expand-file-name invocation-name
>                        invocation-directory))

Great, that should be no concern then.

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Async works by starting a new emacs process.  If that would be an older
version of emacs, that could be a problem.

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi wrote:
> A recent change in master (last 3-4 days) causes async package updates
> using paradox.el to throw this error:
>
> Debugger entered--Lisp error: (wrong-type-argument sequencep #s
> (package-desc swiper [...]

Kaushal has now provided a code snippet that can reproduce this in a
cleanly built emacs -Q with a separate .emacs.d.  In this test case, the
error shows up when installing the second or third package from the
Package Menu.  Curiously, the first installs without a hitch.

It's a bit hard to debug, because the error happens in an async emacs
subprocess.

This code (in paradox-execute.el) is executed in the parent emacs:

  (eval
   `(async-start
     (lambda ()
       (require 'package)
       ,(async-inject-variables "\\`package-")
       (setq package-menu-async nil)
       (dolist (elt package-alist)
         (package-activate (car elt) 'force))
       (let ((alist ,(macroexpand
                      `(paradox--perform-package-transaction
                        ',install-list ',delete-list))))
         (list package-alist
               (when (boundp 'package-selected-packages)
                 package-selected-packages)
               package-archive-contents
               ;; This is the alist that will be passed to the hook.
               (cons '(noquery . ,noquery) (cons '(async . t) alist)))))
     (lambda () ... )))

The first lambda is executed in a subprocess emacs and throws the error
(wrong-type-argument sequencep #s(package-desc swiper ... ))

I'm going to study async.el documentation and code to see if there's a
way to debug this.  But if anyone has any hints, that would be appreciated.

Re: Recent change in master breaks async package update using paradox

[Eli Zaretskii]

> From: Lars Brinkhoff <lars@nocrew.org>
> Date: Fri, 07 Apr 2017 07:42:49 +0200
> 
> This code (in paradox-execute.el) is executed in the parent emacs:
> 
>   (eval
>    `(async-start
>      (lambda ()
>        (require 'package)
>        ,(async-inject-variables "\\`package-")
>        (setq package-menu-async nil)
>        (dolist (elt package-alist)
>          (package-activate (car elt) 'force))
>        (let ((alist ,(macroexpand
>                       `(paradox--perform-package-transaction
>                         ',install-list ',delete-list))))
>          (list package-alist
>                (when (boundp 'package-selected-packages)
>                  package-selected-packages)
>                package-archive-contents
>                ;; This is the alist that will be passed to the hook.
>                (cons '(noquery . ,noquery) (cons '(async . t) alist)))))
>      (lambda () ... )))
> 
> The first lambda is executed in a subprocess emacs and throws the error
> (wrong-type-argument sequencep #s(package-desc swiper ... ))
> 
> I'm going to study async.el documentation and code to see if there's a
> way to debug this.  But if anyone has any hints, that would be appreciated.

What is package-desc?  If its value and/or its semantics are only
valid in the parent Emacs process, then it might not work to use it in
the subprocess.

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Eli Zaretskii wrote:
>> The first lambda is executed in a subprocess emacs and throws the error
>> (wrong-type-argument sequencep #s(package-desc swiper ... ))
>
> What is package-desc?  If its value and/or its semantics are only
> valid in the parent Emacs process, then it might not work to use it in
> the subprocess.

It's a defstruct in package.el.

I interpret the error message as something probably creating a
package-desc instance as a record.  But then something else expects this
instance to be a sequence.  Possibly because it's compiled with the old
cl-defstruct, or maybe it's using a function that worked with vector
structures but now doesn't work with record structures.  (If it's the
latter, we should probably add something to cl-old-struct-compat-mode to
make it work.)

I think there are only a few functions that wants a sequence but not a
record: elt, copy-sequence, concat, reverse, and sort.  In the context
of structure operations, the most likely function would be copy-sequence.

The problem is finding this function call which is made in an async
subprocess emacs.  The backtrace we have only shows what's going in in
the parent emacs.

Re: Recent change in master breaks async package update using paradox

[Eli Zaretskii]

> From: Lars Brinkhoff <lars@nocrew.org>
> Date: Fri, 07 Apr 2017 09:40:39 +0200
> 
> The problem is finding this function call which is made in an async
> subprocess emacs.  The backtrace we have only shows what's going in in
> the parent emacs.

I generally find debugging problems in using async.el to be very hard
indeed.

Debugging problems using async.el

[Lars Brinkhoff]

Eli Zaretskii wrote:
>> From: Lars Brinkhoff <lars@nocrew.org>
>> The problem is finding this function call which is made in an async
>> subprocess emacs.  The backtrace we have only shows what's going in
>> in the parent emacs.
>
> I generally find debugging problems in using async.el to be very hard
> indeed.

I'm beginning to see that!

I set async-debug to t to capture the Lisp form sent to the subordinate
emacs.  I was hoping that I could just wrap this in funcall and run it
in a fresh emacs, but the error doesn't seem to appear when I do that.

I see the subordinate logs stuff to *Messages*, which goes to stdout in
batch mode.  I wonder if that output is saved somewhere.

Re: Debugging problems using async.el

[Kaushal Modi]

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

On Fri, Apr 7, 2017 at 9:29 AM Lars Brinkhoff <lars@nocrew.org> wrote:

> I see the subordinate logs stuff to *Messages*, which goes to stdout in
> batch mode.  I wonder if that output is saved somewhere.
>

It might be useful to add an option to save *Messages* and *Backtrace*
stuff to files on disk.

In async.el, we can see those vars to t and enable the logging to files.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 800 bytes --]

Re: Debugging problems using async.el

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Fri, 07 Apr 2017 14:11:12 +0000
> 
>  I see the subordinate logs stuff to *Messages*, which goes to stdout in
>  batch mode. I wonder if that output is saved somewhere.
> 
> It might be useful to add an option to save *Messages* and *Backtrace* stuff to files on disk.

You can use the 'tee' utility for that.

Re: Debugging problems using async.el

[Lars Brinkhoff]

Kaushal Modi wrote:
> Lars Brinkhoff wrote:
>     I see the subordinate logs stuff to *Messages*, which goes to
>     stdout in batch mode. I wonder if that output is saved somewhere.
>
> It might be useful to add an option to save *Messages* and *Backtrace*
> stuff to files on disk.  In async.el, we can see those vars to t and
> enable the logging to files.

It turns out this isn't necessary.  If you make sure to turn on
async-debug in the subordinate, messages and backtraces are logged in
the parent *emacs* buffer.  I only set async-debug in the parent, so I
didn't get the full debug log.

This was very helpful.  I was able to make good progress towards finding
the root cause.  Not completely there yet though.

Re: Recent change in master breaks async package update using paradox

[Stefan Monnier]

> I think there are only a few functions that wants a sequence but not a
> record: elt, copy-sequence, concat, reverse, and sort.  In the context
> of structure operations, the most likely function would be copy-sequence.

BTW, why not make copy-sequence and elt work on records?
After all, they are "sequences" (of fixed length and of heterogeneous
values, but still).


        Stefan

What is a "sequence"?

[Lars Brinkhoff]

Stefan Monnier wrote:
>> I think there are only a few functions that wants a sequence but not a
>> record: elt, copy-sequence, concat, reverse, and sort.  In the context
>> of structure operations, the most likely function would be copy-sequence.
> 
> BTW, why not make copy-sequence and elt work on records?  After all,
> they are "sequences" (of fixed length and of heterogeneous values, but
> still).

Sure.  I did consider copy-sequence, but then you added copy-record.  I
don't see a great need for elt, but it wouldn't hurt.  Maybe it would be
good for backward compatibility.

In a similar vein, there are also some restrictions on what you can to
to byte-code objects.  You can do aref and concat and even mapcar, but
not aset, and there's no function to make a copy.  I guess a similar
argument could be made that they're "sequences".

Re: What is a "sequence"?

[Stefan Monnier]

> Sure.  I did consider copy-sequence, but then you added copy-record.  I
> don't see a great need for elt, but it wouldn't hurt.

Indeed, maybe `elt` isn't worth it.  But `copy-sequence` would
help backward compatibility.


        Stefan

If records are not sequences, why does aref work on records?

[Paul Eggert]

On 04/07/2017 12:40 AM, Lars Brinkhoff wrote:
> I think there are only a few functions that wants a sequence but not a
> record: elt, copy-sequence, concat, reverse, and sort.

It is confusing that aref works on records but copy-sequence etc. do 
not. This behavior of aref is not documented in the aref doc string, or 
in the Lisp manual under aref.

How about if we define a new function record-ref to be used on records, 
instead of aref? At the low level this would insulate records better 
from the rest of Emacs Lisp, which I think is the intent -- as I 
understand it, records are supposed to be used to create new types, and 
are not supposed to be sorts of aliases for sequences.

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

Paul Eggert wrote:
>> I think there are only a few functions that wants a sequence but not a
>> record: elt, copy-sequence, concat, reverse, and sort.
>
> It is confusing that aref works on records but copy-sequence etc. do
> not. This behavior of aref is not documented in the aref doc string,
> or in the Lisp manual under aref.

You're right.  I can add that documentation, if that's indeed the way
we want to go.

> How about if we define a new function record-ref to be used on
> records, instead of aref? At the low level this would insulate records
> better from the rest of Emacs Lisp, which I think is the intent -- as
> I understand it, records are supposed to be used to create new types,
> and are not supposed to be sorts of aliases for sequences.

That's one way to go, and I understand that point of view.

I believe there is prior art in XEmacs.  They added special functions to
work with byte-code slots, and removed access with aref.  I don't know
the background, but I note that GNU Emacs never followed suit.

The other point of view might be that aref is already there, works fine
for arrays, byte-code, and other pseudovector types, so it might just as
well be extended to work with records too.  Add to this that old code
using aref on vector structures will keep working if passed a record
structure.

To answer the question in the subject, I initially patterned records
mostly after the byte-code type because they are quite alike.  So that
included using aref to access slots.

RE: If records are not sequences, why does aref work on records?

[Drew Adams]

(Caveat: I have not been following this thread.)

If the intention is to add a new kind of sequence, for
example to add a record type and have it be considered
a type of sequence, then I think it should support all
sequence features, just like other sequences.

If it does not support everything intended by "sequence"
then I think we should not present it as a sequence.  In
particular, it should not be documented as a sequence in
that case.

As to what a sequence is, one guide is to refer to Common
Lisp, which carefully defines sequences and sequence
operations.

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

Drew Adams wrote:
> If the intention is to add a new kind of sequence, for
> example to add a record type and have it be considered
> a type of sequence, then I think it should support all
> sequence features, just like other sequences.
>
> If it does not support everything intended by "sequence"
> then I think we should not present it as a sequence.  In
> particular, it should not be documented as a sequence in
> that case.
>
> As to what a sequence is, one guide is to refer to Common
> Lisp, which carefully defines sequences and sequence
> operations.

So what do you think about the byte-code type, which supports many, but
not all, sequence operations?  Is that a long-standing bug?

RE: If records are not sequences, why does aref work on records?

[Drew Adams]

> > If the intention is to add a new kind of sequence, for
> > example to add a record type and have it be considered
> > a type of sequence, then I think it should support all
> > sequence features, just like other sequences.
> >
> > If it does not support everything intended by "sequence"
> > then I think we should not present it as a sequence.  In
> > particular, it should not be documented as a sequence in
> > that case.
> >
> > As to what a sequence is, one guide is to refer to Common
> > Lisp, which carefully defines sequences and sequence
> > operations.
> 
> So what do you think about the byte-code type, which supports many, but
> not all, sequence operations?  Is that a long-standing bug?

I know nothing about it; sorry.  If it is then it is.

Emacs, like Common Lisp, should have a well-defined sequence
type.  We aren't required to model ours on CL's, but other
things being equal it wouldn't be a bad idea to look to CL
for guidance/inspiration.

Just one opinion.

Re: If records are not sequences, why does aref work on records?

[Noam Postavsky]

On Fri, Apr 7, 2017 at 2:30 PM, Drew Adams <drew.adams@oracle.com> wrote:
>>
>> So what do you think about the byte-code type, which supports many, but
>> not all, sequence operations?  Is that a long-standing bug?
>
> I know nothing about it; sorry.  If it is then it is.
>
> Emacs, like Common Lisp, should have a well-defined sequence
> type.  We aren't required to model ours on CL's, but other
> things being equal it wouldn't be a bad idea to look to CL
> for guidance/inspiration.

AFAIK, CL's structures (if defined without a :type argument) are not
sequences, and I don't see any need for Elisp's to be either.

To quote the elisp manual: "The common property that all sequences
have is that each
is an ordered collection of elements." A record's elements are more
like an unordered set. If you do (cl-defstruct person age height), you
don't care whether 'age' goes before 'height', just that person-age
gives you the age field. And some sequence operations make no sense on
records: e.g., doing 'sort' or 'reverse' on a record would not result
in a valid record.

If we decide to support aref or copy-sequence on records for
efficiency or backwards compatibility that doesn't mean we want them
to be full sequences.

Re: If records are not sequences, why does aref work on records?

[Stefan Monnier]

> If we decide to support aref or copy-sequence on records for
> efficiency or backwards compatibility that doesn't mean we want them
> to be full sequences.

FWIW, I agree.  I think we do need aref and copy-sequence to work on
them, for backward compatibility reasons, but `sequencep` should
return nil.


        Stefan

RE: If records are not sequences, why does aref work on records?

[Drew Adams]

> > If we decide to support aref or copy-sequence on records for
> > efficiency or backwards compatibility that doesn't mean we want them
> > to be full sequences.
> 
> FWIW, I agree.  I think we do need aref and copy-sequence to work on
> them, for backward compatibility reasons, but `sequencep` should
> return nil.

If such an approach is taken, with `copy-sequence' repurposed
instead of having a new function to do the job, then its
doc will need to be changed, to clarify that the arg and the
result are not necessarily sequences.

I don't know why we would want to pollute it that way.
Let `copy-sequence' be a sequence function.

I don't understand the backward-compatibility argument.
A record type has not existed before.  When/where do we
already have `aref'/`copy-sequence' being applied to
something other than a sequence/array?

Oh, I see.  Someone long ago let `aref' apply to byte code.
Too bad.  But what about `copy-sequence' - what backward
compatibility is involved there?

And why does `aref' apply to byte code?  Should byte code
perhaps be deemed an array (satisfy `arrayp')?

Or should we add a new function for byte code, to replace
the use of `aref' - and deprecate the latter?  Seems like
`aref', just as for Common Lisp, should apply only to an
array.

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-07 16:49, Drew Adams wrote:
> I don't understand the backward-compatibility argument.
> A record type has not existed before.  When/where do we
> already have `aref'/`copy-sequence' being applied to
> something other than a sequence/array?

Aren't record types used by cl-defstruct now?

Re: If records are not sequences, why does aref work on records?

[Noam Postavsky]

On Fri, Apr 7, 2017 at 4:49 PM, Drew Adams <drew.adams@oracle.com> wrote:
>
> I don't understand the backward-compatibility argument.
> A record type has not existed before.  When/where do we
> already have `aref'/`copy-sequence' being applied to
> something other than a sequence/array?

Up until the recent changes, the things produced by cl-defstruct
make-foo functions were vectors with the first element being the
symbol 'cl-struct-foo'. So they could be indexed with aref and copied
with copy-sequence. Now they are "record" objects, but we don't want
to break code that uses aref and copy-sequence on them.

Re: If records are not sequences, why does aref work on records?

[Paul Eggert]

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

On 04/07/2017 02:02 PM, Noam Postavsky wrote:
> Now they are "record" objects, but we don't want
> to break code that uses aref and copy-sequence on them.

OK, given what you and Stefan have said, for backward-compatibility 
reasons copy-sequence should work on records. This means Emacs doesn't 
need a copy-record function, as people can just use copy-sequence. I 
installed the attached patch to do that, and to try to document the 
current low-level behavior better.


[-- Attachment #2: 0001-Deprecate-copy-record-in-favor-of-copy-sequence.patch --]
[-- Type: application/x-patch, Size: 10157 bytes --]

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

Paul Eggert <eggert@cs.ucla.edu> writes:
> OK, given what you and Stefan have said, for backward-compatibility
> reasons copy-sequence should work on records. This means Emacs doesn't
> need a copy-record function, as people can just use copy-sequence. I
> installed the attached patch to do that, and to try to document the
> current low-level behavior better.

Some EIEIO tests now fail.  I haven't yet checked if it's because of
this particular change, but it seems a likely candidate.

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

> Some EIEIO tests now fail.  I haven't yet checked if it's because of
> this particular change, but it seems a likely candidate.

Nevern mind!  A bootstrap fixed that.

Re: If records are not sequences, why does aref work on records?

[Noam Postavsky]

On Fri, Apr 7, 2017 at 9:58 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> This means Emacs doesn't need a
> copy-record function, as people can just use copy-sequence.

I think having copy-record would still be a good idea, as it helps
show the intent better. Possibly just as an alias of copy-sequence, or
perhaps as a wrapper function which would signal error for
non-records?

Re: If records are not sequences, why does aref work on records?

[Paul Eggert]

Noam Postavsky wrote:
> having copy-record would still be a good idea, as it helps
> show the intent better.

If we go that route, for consistency shouldn't Emacs also have aref-record and 
aref-byte-code-function as aliases for 'aref'?

A couple more glitches in current master, while we're at it: (aref 1 2) signals 
(wrong-type-argument arrayp 1) even though aref's first argument need not be an 
array. Similarly, (copy-sequence 1) signals (wrong-type-argument sequencep 1) 
even though copy-sequence's first argument need not be a sequence.

Re: If records are not sequences, why does aref work on records?

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 8 Apr 2017 00:42:57 -0700
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> Noam Postavsky wrote:
> > having copy-record would still be a good idea, as it helps
> > show the intent better.
> 
> If we go that route, for consistency shouldn't Emacs also have aref-record and 
> aref-byte-code-function as aliases for 'aref'?

Let me remind you that this kind of "literal" consistency was never a
goal in Emacs development.  It's one of the first lessons I got from
RMS years ago; I'm sure you've heard that as well at some point.  We
make stuff consistent where it makes sense, but not everywhere.

Re: If records are not sequences, why does aref work on records?

[Noam Postavsky]

On Sat, Apr 8, 2017 at 3:42 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Noam Postavsky wrote:
>>
>> having copy-record would still be a good idea, as it helps
>> show the intent better.
>
>
> If we go that route, for consistency shouldn't Emacs also have aref-record

Well, if you're using aref on a record, the record abstraction is
already being broken, so it doesn't seem as useful. On the other hand,
I had forgotten about the copy-foo function that cl-defstruct creates,
so maybe copy-record really is redundant.

Re: If records are not sequences, why does aref work on records?

[Philipp Stephani]

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

Noam Postavsky <npostavs@users.sourceforge.net> schrieb am Fr., 7. Apr.
2017 um 23:03 Uhr:

> On Fri, Apr 7, 2017 at 4:49 PM, Drew Adams <drew.adams@oracle.com> wrote:
> >
> > I don't understand the backward-compatibility argument.
> > A record type has not existed before.  When/where do we
> > already have `aref'/`copy-sequence' being applied to
> > something other than a sequence/array?
>
> Up until the recent changes, the things produced by cl-defstruct
> make-foo functions were vectors with the first element being the
> symbol 'cl-struct-foo'. So they could be indexed with aref and copied
> with copy-sequence. Now they are "record" objects, but we don't want
> to break code that uses aref and copy-sequence on them.
>
>
While the manual mentions that vectors or lists are used, it doesn't say
that the sequence functions can simply be used. Rather, the implementation
as sequence is an implementation detail that users shouldn't rely on.
cl-defstruct even creates a copy-FOO function to copy structures, which
should be used instead of copy-sequence. Therefore, I think it's fine to
not allow copy-sequence and aref on structure types any more, if this
change is properly documented.

[-- Attachment #2: Type: text/html, Size: 1899 bytes --]

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-08 07:34, Philipp Stephani wrote:
> While the manual mentions that vectors or lists are used, it doesn't 
> say that the sequence functions can simply be used.

It doesn't need to: since vectors and lists are used, it's natural to use vector and list functions on these objects.  It goes further: the cl-struct predicates are explicitly *documented* to work fine on vectors:

     Given any Lisp object X, ‘(person-p X)’ returns true if X looks
     like a ‘person’, and false otherwise.  (Again, in Common Lisp this
     predicate would be exact; in Emacs Lisp the best it can do is
     verify that X is a vector of the correct length that starts with
     the correct tag symbol.)

… and the :type predicate says:

    Vectors are used by
    default, but ‘(:type list)’ will cause structure objects to be
    stored as lists instead.

… and of course, until the changes discussed here, cl-defstructs were printed just like vectors.

> Rather, the implementation as sequence is an implementation detail
> that users shouldn't rely on.

The manual doesn't say that — I don't think it's fair to call "implementation detail" something that's documented and explicitly not described as an implementation detail:

    Since the underlying Emacs Lisp system
    provides no way to create new distinct types, this package implements
    structures as vectors (or lists upon request) with a special “tag”
    symbol to identify them.

> cl-defstruct even creates a copy-FOO function to copy structures,
> which should be used instead of copy-sequence. Therefore, I think
> it's fine to not allow copy-sequence and aref on structure types any
> more, if this change is properly documented.

Please, let's not break perfectly working code gratuitously.

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

Clément Pit-Claudel wrote:
> It doesn't need to: since vectors and lists are used, it's natural to
> use vector and list functions on these objects.  It goes further: the
> cl-struct predicates are explicitly *documented* to work fine on
> vectors:
>
>      Given any Lisp object X, ‘(person-p X)’ returns true if X looks
>      like a ‘person’, and false otherwise.  (Again, in Common Lisp this
>      predicate would be exact; in Emacs Lisp the best it can do is
>      verify that X is a vector of the correct length that starts with
>      the correct tag symbol.)

Not any more.  When I made my change, I found this part of the manual
and deleted the text in parentheses.  And also changed much of the other
places you quoted.

This is not to say that I weigh in on any side of this debate.  I think
both sides make good arguments.

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-08 11:24, Lars Brinkhoff wrote:
> Clément Pit-Claudel wrote:
>> It doesn't need to: since vectors and lists are used, it's natural to
>> use vector and list functions on these objects.  It goes further: the
>> cl-struct predicates are explicitly *documented* to work fine on
>> vectors:
>>
>>      Given any Lisp object X, ‘(person-p X)’ returns true if X looks
>>      like a ‘person’, and false otherwise.  (Again, in Common Lisp this
>>      predicate would be exact; in Emacs Lisp the best it can do is
>>      verify that X is a vector of the correct length that starts with
>>      the correct tag symbol.)
> 
> Not any more.  When I made my change, I found this part of the manual
> and deleted the text in parentheses.  And also changed much of the other
> places you quoted.

Yup, of course; it was natural to update the manual along with the change.  I was pointing to the documentation as it was when people introduced the `arefs' and 'cl-defstruct' whose validity is now under discussion in my their code, because the question was whether people should have known that using `aref' this way was relying on an implementation detail.

Re: If records are not sequences, why does aref work on records?

[Lars Brinkhoff]

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
>> Not any more.  When I made my change, I found this part of the manual
>> and deleted the text in parentheses.  And also changed much of the
>> other places you quoted.
>
> Yup, of course; it was natural to update the manual along with the
> change.  I was pointing to the documentation as it was when people
> introduced the `arefs' and 'cl-defstruct' whose validity is now under
> discussion in my their code, because the question was whether people
> should have known that using `aref' this way was relying on an
> implementation detail.

Oh, I see.  Yes, I agree with your point.

Re: If records are not sequences, why does aref work on records?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

In principle, there is no reason for vector functions to work on a
defstruct.  We can declare that usage obsolete, while keeping it
functioning.  In 3 years we can make it generate an error which
is easy to turn off, to nudge people to change the code.

On the other hand, it is wasteful to create myriad 'copy-FOO'
functions that all do the same thing.  We may as well have 
one function 'copy-struct' function that copies all kinds of sequences.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: If records are not sequences, why does aref work on records?

[Stefan Monnier]

> On the other hand, it is wasteful to create myriad 'copy-FOO'
> functions that all do the same thing.  We may as well have
> one function 'copy-struct' function that copies all kinds of sequences.

The only reason cl-defstruct defines `copy-FOO` is because Common-Lisp's
defstruct does.  I can't remember ever seeing Elisp code make use of those
"copiers".


        Stefan

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-07 15:07, Noam Postavsky wrote:
> If you do (cl-defstruct person age height), you
> don't care whether 'age' goes before 'height', just that person-age
> gives you the age field.

Sometimes you do — for example, you might know that multiple structures in your application have the same data in their first field, and if so (aref x 1) should work.
It's a bit like hashtables: if I have multiple hashtables that all have an "age" key, I can access that key independently of what else is in the table.

Re: If records are not sequences, why does aref work on records?

[Noam Postavsky]

On Fri, Apr 7, 2017 at 3:25 PM, Clément Pit-Claudel
<cpitclaudel@gmail.com> wrote:
> On 2017-04-07 15:07, Noam Postavsky wrote:
>> If you do (cl-defstruct person age height), you
>> don't care whether 'age' goes before 'height', just that person-age
>> gives you the age field.
>
> Sometimes you do — for example, you might know that multiple structures in your application have the same data in their first field, and if so (aref x 1) should work.

In that case, you should be probably be using :include for the common
parts, or else :type to explicitly use vectors. I'm sure you can come
up with exceptions to that too though :)

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-07 15:41, Noam Postavsky wrote:
> On Fri, Apr 7, 2017 at 3:25 PM, Clément Pit-Claudel
> <cpitclaudel@gmail.com> wrote:
>> On 2017-04-07 15:07, Noam Postavsky wrote:
>>> If you do (cl-defstruct person age height), you
>>> don't care whether 'age' goes before 'height', just that person-age
>>> gives you the age field.
>>
>> Sometimes you do — for example, you might know that multiple structures in your application have the same data in their first field, and if so (aref x 1) should work.
> 
> In that case, you should be probably be using :include for the common
> parts, or else :type to explicitly use vectors. 

Very good point.

> I'm sure you can come up with exceptions to that too though :)

Hum.  Users who don't read the manual closely enough ? 😳 :)

Re: If records are not sequences, why does aref work on records?

[Stefan Monnier]

> Sometimes you do — for example, you might know that multiple structures in
> your application have the same data in their first field, and if so (aref
> x 1) should work.

In most such cases, you (c|sh)ould have defined a shared parent struct, tho.


        Stefan

Re: If records are not sequences, why does aref work on records?

[Clément Pit-Claudel]

On 2017-04-07 12:56, Paul Eggert wrote:
> How about if we define a new function record-ref to be used on
> records, instead of aref? At the low level this would insulate
> records better from the rest of Emacs Lisp, which I think is the
> intent -- as I understand it, records are supposed to be used to
> create new types, and are not supposed to be sorts of aliases for
> sequences.

Won't this break things, though? I thought cl-defstruct now produced record, so will all code that used aref on cl-defstructs need to be updated?

(I use aref on things defined with cl-defstruct from time to time. The latest example was a function on binary trees, defined as a union type of two records (one type of record for branches, and one type of record for leaves).  Both types carried the same information in the first slot, which allowed me to replace (if (branch-p x) (branch-v x) (leaf-v x)) by (aref x 1).  This made a difference, performance-wise.)

Cheers,
Clément.

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi wrote:
> A recent change in master (last 3-4 days) causes async package updates
> using paradox.el to throw this error:
>
> Debugger entered--Lisp error: (wrong-type-argument sequencep #s
> (package-desc swiper (20170328 737) "Isearch with an overview. Oh,
> man!" ((emacs (24 1)) (ivy (0 8 0))) nil nil
> "/home/kmodi/.emacs.d/elpa_26/swiper-20170328.737" ((:keywords
> "matching") (:url . "https://github.com/abo-abo/swiper") (:stars #
> ("249" 0 3 (font-lock-face paradox-starred-face)))) nil))

I have now narrowed it down to this:

  (read "(#1=#s(foo) #1#)")

I.e. the problem is that the #1= syntax doesn't support records.  This
shouldn't be hard to fix.

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

> I have now narrowed it down to this:
>
>   (read "(#1=#s(foo) #1#)")
>
> I.e. the problem is that the #1= syntax doesn't support records.  This
> shouldn't be hard to fix.

I have pushed a fix: 4fbfd7a

Re: Recent change in master breaks async package update using paradox

[Clément Pit-Claudel]

On 2017-04-08 02:50, Lars Brinkhoff wrote:
>> I have now narrowed it down to this:
>>
>>   (read "(#1=#s(foo) #1#)")
>>
>> I.e. the problem is that the #1= syntax doesn't support records.  This
>> shouldn't be hard to fix.
> 
> I have pushed a fix: 4fbfd7a

Thanks a lot for your hard work on this cool new feature!

Re: Recent change in master breaks async package update using paradox

[Kaushal Modi]

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

On Sat, Apr 8, 2017, 2:50 AM Lars Brinkhoff <lars@nocrew.org> wrote:

> > I have now narrowed it down to this:
> >
> >   (read "(#1=#s(foo) #1#)")
> >
> > I.e. the problem is that the #1= syntax doesn't support records.  This
> > shouldn't be hard to fix.
>
> I have pushed a fix: 4fbfd7a
>

Thanks a lot for the debug and fix. This was a tough bug to discover,
especially when working with async. If you don't mind, and when you have
time, can you please write about how you went about to debug this issue?

Unfortunately, I don't have access to my emacs build machine this weekend.
I will this out the first thing on Monday!

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1352 bytes --]

Re: Recent change in master breaks async package update using paradox

[Lars Brinkhoff]

Kaushal Modi wrote:
> Thanks a lot for the debug and fix. This was a tough bug to discover,
> especially when working with async. If you don't mind, and when you
> have time, can you please write about how you went about to debug this
> issue?

Well, half the credit should go to yourself for providing a good test
case for reproducing the problem.

Your backtrace pointed to the call to async-start in paradoc-execute.el.
The problem was knowing what happened in the subordinate Emacs.  I kind
of expected there to be some kind of debugging tool, so I keept my eyes
peeled for that.  Sure enough, there was async-debug.  First I only
turned it on in the parent Emacs and got some intersting info.  But the
breakthrough came when I turned it on also in the subordinate.  Then I
got the full backtrace pinpointing the location of the error.

It turned out that the error came from reading a huge string.  So it
still wasn't obvious what part of the string was wrong.  But it was just
a matter of removing substrings until only the one signalling an error
remained.