[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#71116: 30.0.50; comp-normalize-valset doesn't sort consistently
From: |
Andrea Corallo |
Subject: |
bug#71116: 30.0.50; comp-normalize-valset doesn't sort consistently |
Date: |
Wed, 22 May 2024 15:10:59 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Daniel Clemente <n142857@gmail.com> writes:
> Current code from comp-cstr.el:
>
> (defun comp-normalize-valset (valset)
> "Sort and remove duplicates from VALSET then return it."
> (cl-sort (cl-remove-duplicates valset :test #'eq)
> (lambda (x y)
> (cond
> ((and (symbolp x) (symbolp y))
> (string< x y))
> ((and (symbolp x) (not (symbolp y)))
> t)
> ((and (not (symbolp x)) (symbolp y))
> nil)
> ((or (consp x) (consp y)
> nil))
> (t
> (< (sxhash-equal x)
> (sxhash-equal y)))))))
>
> This part:
> ((or (consp x) (consp y)
> nil))
>
> Seems like a typo; as if this was intended:
> ((or (consp x) (consp y))
> nil)
>
> In practice, it means it's not sorting well. The presence of a cons can even
> change how the other elements are sorted:
>
> ;; This produces: ((a . 1) 2 3)
> (comp-normalize-valset '(
> 2
> 3
> (a . 1)
> ))
>
> ;; This produces: (2 3 (a . 1))
> (comp-normalize-valset '(
> (a . 1)
> 2
> 3
> ))
>
> ;; This produces: (3 (a . 1) 2)
> (comp-normalize-valset '(
> 2
> (a . 1)
> 3
> ))
>
> Since all three examples use a list with the same elements, I would expect
> the same result after sorting: a sorted list
> (by some definition of sorted). Otherwise the function documentation must be
> adjusted.
>
> I'm just reporting this because I was reading new code and found this part
> hard to understand. I'm not familiar with the
> comp-cstr.el code or with how this affects native compilation, or whether
> there's any bug. My example doesn't represent
> how the actual code is used.
>
> For context, the original intention was to avoid comparing conses with
> sxhash-equal.
> https://lists.gnu.org/archive/html/emacs-devel/2024-02/msg00406.html
Yes this is my todo list, I think for how the code is now sorting should
not even be necessary anymore, so I want to give it a try at remove it
entirely.
Andrea