bug#71116: 30.0.50; comp-normalize-valset doesn't sort consistently

[Andrea Corallo <acorallo@gnu.org>]

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