* behaviour change in cl-subseq
@ 2015-08-05 12:14 Phillip Lord
2015-08-06 0:10 ` Leo Liu
2015-08-06 8:08 ` Nicolas Petton
0 siblings, 2 replies; 14+ messages in thread
From: Phillip Lord @ 2015-08-05 12:14 UTC (permalink / raw
To: emacs-devel
I've just had a bug report for pabbrev. The root cause is a change in
the implementation of cl-subseq.
In Emacs 24:
(cl-subseq '() 10 10) ;; nil
While in Emacs 25
In Emacs 25 `cl-subseq' has been redefined in terms of seq-subseq.
(seq-subseq '() 10 10) ;; errors
(cl-subseq '() 10 10) ;; errors
Which is a reasonably substantial change in the interface of cl-subseq.
The actual (exceptional) behaviour of cl-subseq is not documented, so
it's reasonable, but perhaps not sensible. It's easy to fix in pabbrev
(and anywhere) in a way which does not require me to probe for Emacs
versions.
I offer no opinions, just wanted to check whether this was intended.
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-05 12:14 behaviour change in cl-subseq Phillip Lord
@ 2015-08-06 0:10 ` Leo Liu
2015-08-06 6:11 ` Tassilo Horn
2015-08-06 9:34 ` Phillip Lord
2015-08-06 8:08 ` Nicolas Petton
1 sibling, 2 replies; 14+ messages in thread
From: Leo Liu @ 2015-08-06 0:10 UTC (permalink / raw
To: emacs-devel
On 2015-08-05 20:14 +0800, Phillip Lord wrote:
> (cl-subseq '() 10 10) ;; errors
What did you get when you run it in Common Lisp?
Leo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-06 0:10 ` Leo Liu
@ 2015-08-06 6:11 ` Tassilo Horn
2015-08-06 9:34 ` Phillip Lord
1 sibling, 0 replies; 14+ messages in thread
From: Tassilo Horn @ 2015-08-06 6:11 UTC (permalink / raw
To: Leo Liu; +Cc: emacs-devel
Leo Liu <sdl.web@gmail.com> writes:
>> (cl-subseq '() 10 10) ;; errors
>
> What did you get when you run it in Common Lisp?
In SBCL 1.2.12 I get an error:
* (subseq '() 10 10)
debugger invoked on a SB-KERNEL:BOUNDING-INDICES-BAD-ERROR in thread
#<THREAD "main thread" RUNNING {1002D76633}>:
The bounding indices 10 and 10 are bad for a sequence of length 0.
See also:
The ANSI Standard, Glossary entry for "bounding index designator"
The ANSI Standard, writeup for Issue SUBSEQ-OUT-OF-BOUNDS:IS-AN-ERROR
So I guess the new behavior is in compliance with Common Lisp. Or well,
not with the current ANSI standard but with a future one.
Bye,
Tassilo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-06 0:10 ` Leo Liu
2015-08-06 6:11 ` Tassilo Horn
@ 2015-08-06 9:34 ` Phillip Lord
2015-08-06 9:48 ` Leo Liu
1 sibling, 1 reply; 14+ messages in thread
From: Phillip Lord @ 2015-08-06 9:34 UTC (permalink / raw
To: Leo Liu; +Cc: emacs-devel
Leo Liu <sdl.web@gmail.com> writes:
> On 2015-08-05 20:14 +0800, Phillip Lord wrote:
>> (cl-subseq '() 10 10) ;; errors
>
> What did you get when you run it in Common Lisp?
Fortunately, Tassilo has answered this; I've never used CL.
I agree that this change is entirely justifiable. I'm generally not a
huge fan of silently ignoring inaccurate arguments. If you are happy
with the changed interface, then I have no major problem, although it
may break packages besides mine.
Do you mind if I patch the docstrings, though?
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-06 9:34 ` Phillip Lord
@ 2015-08-06 9:48 ` Leo Liu
2015-08-07 14:00 ` Phillip Lord
0 siblings, 1 reply; 14+ messages in thread
From: Leo Liu @ 2015-08-06 9:48 UTC (permalink / raw
To: emacs-devel
On 2015-08-06 17:34 +0800, Phillip Lord wrote:
> Fortunately, Tassilo has answered this; I've never used CL.
I answered it in a hurry before going to work :(
> I agree that this change is entirely justifiable. I'm generally not a
> huge fan of silently ignoring inaccurate arguments. If you are happy
> with the changed interface, then I have no major problem, although it
> may break packages besides mine.
The old behaviour was odd and inconsistent (try it on []).
> Do you mind if I patch the docstrings, though?
Not at all.
Leo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-06 9:48 ` Leo Liu
@ 2015-08-07 14:00 ` Phillip Lord
2015-08-07 14:26 ` Nicolas Petton
0 siblings, 1 reply; 14+ messages in thread
From: Phillip Lord @ 2015-08-07 14:00 UTC (permalink / raw
To: Leo Liu; +Cc: emacs-devel
Leo Liu <sdl.web@gmail.com> writes:
> On 2015-08-06 17:34 +0800, Phillip Lord wrote:
>> Fortunately, Tassilo has answered this; I've never used CL.
>
> I answered it in a hurry before going to work :(
>
>> I agree that this change is entirely justifiable. I'm generally not a
>> huge fan of silently ignoring inaccurate arguments. If you are happy
>> with the changed interface, then I have no major problem, although it
>> may break packages besides mine.
>
> The old behaviour was odd and inconsistent (try it on []).
>
>> Do you mind if I patch the docstrings, though?
>
> Not at all.
Was working through this, and found this behaviour.
;; error
(seq-subseq '() 10 10)
;; nil
(seq-subseq '() -10 -10)
So, a bounding box error when the index is too large, but it is silently
ignored when too low.
Not convinced this makes sense.
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-07 14:00 ` Phillip Lord
@ 2015-08-07 14:26 ` Nicolas Petton
2015-08-07 15:51 ` Phillip Lord
2015-08-20 20:50 ` Phillip Lord
0 siblings, 2 replies; 14+ messages in thread
From: Nicolas Petton @ 2015-08-07 14:26 UTC (permalink / raw
To: Phillip Lord, Leo Liu; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 449 bytes --]
Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
> Was working through this, and found this behaviour.
>
> ;; error
> (seq-subseq '() 10 10)
>
> ;; nil
> (seq-subseq '() -10 -10)
>
> So, a bounding box error when the index is too large, but it is silently
> ignored when too low.
>
> Not convinced this makes sense.
Indeed, it doesn't. Do you want to push a patch for it?
Nico
--
Nicolas Petton
http://nicolas-petton.fr
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-07 14:26 ` Nicolas Petton
@ 2015-08-07 15:51 ` Phillip Lord
2015-08-07 16:08 ` Nicolas Petton
2015-08-20 20:50 ` Phillip Lord
1 sibling, 1 reply; 14+ messages in thread
From: Phillip Lord @ 2015-08-07 15:51 UTC (permalink / raw
To: Nicolas Petton; +Cc: Leo Liu, emacs-devel
Nicolas Petton <nicolas@petton.fr> writes:
> Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
>
>> Was working through this, and found this behaviour.
>>
>> ;; error
>> (seq-subseq '() 10 10)
>>
>> ;; nil
>> (seq-subseq '() -10 -10)
>>
>> So, a bounding box error when the index is too large, but it is silently
>> ignored when too low.
>>
>> Not convinced this makes sense.
>
> Indeed, it doesn't. Do you want to push a patch for it?
Yes, will do. My expectation is this:
;; error
(seq-subseq '() 10 10)
;; (same) error
(seq-subseq '() -10 -10)
You agree?
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-07 15:51 ` Phillip Lord
@ 2015-08-07 16:08 ` Nicolas Petton
2015-08-07 21:23 ` Phillip Lord
0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Petton @ 2015-08-07 16:08 UTC (permalink / raw
To: Phillip Lord; +Cc: Leo Liu, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 213 bytes --]
Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
> Yes, will do. My expectation is this:
>
> ;; error
> (seq-subseq '() 10 10)
>
> ;; (same) error
> (seq-subseq '() -10 -10)
>
>
> You agree?
Yes, I do.
Nico
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-07 16:08 ` Nicolas Petton
@ 2015-08-07 21:23 ` Phillip Lord
2015-08-08 19:56 ` Nicolas Petton
0 siblings, 1 reply; 14+ messages in thread
From: Phillip Lord @ 2015-08-07 21:23 UTC (permalink / raw
To: Nicolas Petton; +Cc: Leo Liu, emacs-devel
Nicolas Petton <nicolas@petton.fr> writes:
> Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
>
>> Yes, will do. My expectation is this:
>>
>> ;; error
>> (seq-subseq '() 10 10)
>>
>> ;; (same) error
>> (seq-subseq '() -10 -10)
>>
>>
>> You agree?
>
> Yes, I do.
Added as fix/subsequence-error-with-negative-sequences branching from
current trunk.
Would you like to check that I have got everything in place? Please feel
free to merge/rebase to trunk iff you are happy. If not, let me know and
I will improve.
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-07 14:26 ` Nicolas Petton
2015-08-07 15:51 ` Phillip Lord
@ 2015-08-20 20:50 ` Phillip Lord
2015-08-21 0:02 ` Artur Malabarba
1 sibling, 1 reply; 14+ messages in thread
From: Phillip Lord @ 2015-08-20 20:50 UTC (permalink / raw
To: Nicolas Petton; +Cc: Emacs-Devel devel
Nico
I'm wondering about this function.
(defun seq-contains-p (seq elt &optional testfn)
"Return the first element in SEQ that equals to ELT.
Equality is defined by TESTFN if non-nil or by `equal' if nil."
(seq-some-p (lambda (e)
(funcall (or testfn #'equal) elt e))
seq))
Two issues. Implementation wise, you are depending on behaviour outside
the interface of seq-some-p which says....
"Return any element for which (PRED element) is non-nil in SEQ, nil
otherwise."
which is "any" not "first". Minor point, and technically an
implementation detail.
But the more serious problem is this...
;; => nil
(seq-contains-p '(1 2 3) nil)
;; => nil
(seq-contains-p '(1 2 nil) nil)
Which is correct by the documentation of seq-contains-p but not actually
any use. Why not have it return nil or t?
;; => nil
(seq-contains-p '(1 2 3) nil)
;; => t
(seq-contains-p '(1 2 nil) nil)
As it happens, that forces the implementation issue as well (because you
can use `seq-some-p' any more for the same reason).
The other option is to add a `seq-contains-nil-p' function.
Personally, I'd change seq-contains-p. Better to break the interface of
seq-contains-p earlier rather than later.
Phil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-20 20:50 ` Phillip Lord
@ 2015-08-21 0:02 ` Artur Malabarba
0 siblings, 0 replies; 14+ messages in thread
From: Artur Malabarba @ 2015-08-21 0:02 UTC (permalink / raw
To: Phillip Lord; +Cc: Nicolas Petton, Emacs-Devel devel
> (defun seq-contains-p (seq elt &optional testfn)
> "Return the first element in SEQ that equals to ELT.
> Equality is defined by TESTFN if non-nil or by `equal' if nil."
> (seq-some-p (lambda (e)
> (funcall (or testfn #'equal) elt e))
> seq))
>
> Two issues. Implementation wise, you are depending on behaviour outside
> the interface of seq-some-p which says....
>
> "Return any element for which (PRED element) is non-nil in SEQ, nil
> otherwise."
>
> The other option is to add a `seq-contains-nil-p' function.
>
> Personally, I'd change seq-contains-p. Better to break the interface of
> seq-contains-p earlier rather than later.
I think both functions need to change behavior. They are named as
`-p', but "return an element" is not the behavior of a boolean
predicate (precisely because of the situation you describe).
IMO, the current behavior of `seq-some-p' should be called `seq-some'.
And the same for `seq-contains'.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: behaviour change in cl-subseq
2015-08-05 12:14 behaviour change in cl-subseq Phillip Lord
2015-08-06 0:10 ` Leo Liu
@ 2015-08-06 8:08 ` Nicolas Petton
1 sibling, 0 replies; 14+ messages in thread
From: Nicolas Petton @ 2015-08-06 8:08 UTC (permalink / raw
To: Phillip Lord, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 950 bytes --]
Phillip Lord <phillip.lord@newcastle.ac.uk> writes:
> I've just had a bug report for pabbrev. The root cause is a change in
> the implementation of cl-subseq.
>
> In Emacs 24:
>
> (cl-subseq '() 10 10) ;; nil
>
> While in Emacs 25
>
> In Emacs 25 `cl-subseq' has been redefined in terms of seq-subseq.
>
> (seq-subseq '() 10 10) ;; errors
> (cl-subseq '() 10 10) ;; errors
>
> Which is a reasonably substantial change in the interface of cl-subseq.
> The actual (exceptional) behaviour of cl-subseq is not documented, so
> it's reasonable, but perhaps not sensible. It's easy to fix in pabbrev
> (and anywhere) in a way which does not require me to probe for Emacs
> versions.
>
> I offer no opinions, just wanted to check whether this was intended.
Hi,
The change was apparently intended, and was done in commit
253d44bd27b7d90b614b6b968a3b125eeb0a48f2
Cheers,
Nico
--
Nicolas Petton
http://nicolas-petton.fr
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-08-21 0:02 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-05 12:14 behaviour change in cl-subseq Phillip Lord
2015-08-06 0:10 ` Leo Liu
2015-08-06 6:11 ` Tassilo Horn
2015-08-06 9:34 ` Phillip Lord
2015-08-06 9:48 ` Leo Liu
2015-08-07 14:00 ` Phillip Lord
2015-08-07 14:26 ` Nicolas Petton
2015-08-07 15:51 ` Phillip Lord
2015-08-07 16:08 ` Nicolas Petton
2015-08-07 21:23 ` Phillip Lord
2015-08-08 19:56 ` Nicolas Petton
2015-08-20 20:50 ` Phillip Lord
2015-08-21 0:02 ` Artur Malabarba
2015-08-06 8:08 ` Nicolas Petton
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.