bug#61521: "default" is now the first item returned from (font-faces), breaking various code.

[Brennan Vincent]

On 2023-02-15 09:24, Eli Zaretskii wrote:
>> From: Brennan Vincent <brennan@umanwizard.com>
>> Date: Wed, 15 Feb 2023 09:01:31 -0500
>> Cc: 61521@debbugs.gnu.org
>>
>>> On Feb 15, 2023, at 07:58, Eli Zaretskii <eliz@gnu.org> wrote:
>>>
>>> 
>>>>
>>>> Date: Tue, 14 Feb 2023 19:31:30 -0500
>>>> From: Brennan Vincent <brennan@umanwizard.com>
>>>>
>>>> Various code seems to expect "default" to be the /last/ item in the list
>>>> returned by that function, not the first. For example, this comment in 
>>>> faces.el:
>>>>
>>>>  ;; The `reverse' is so that `default' goes first.
>>>>  (dolist (face (nreverse (face-list)))
>>>
>>> That comment is obsolete and needs to be changed (and the call to
>>> nreverse should perhaps be removed).
>>
>> If the order returned by face-list is not guaranteed, then why does it do 
>> sorting at all?
> 
> Good question.  AFAICT, the sorting was added when we switched from
> storing faces in alists to storing them in hash tables.  It probably
> sorted faces to be more compatible with what face-list returned before
> the switch to hash table.  So I suspect the order we have now is
> simply a bug, and we do need to change the order of sorting to get
> back the original order.

I tend to agree. Sorry for not explaining this reasoning more fully in my
original message.

Here's what I suspect happened (not 100% sure, it's just a theory):

(1) Initially set of faces was stored as a list, so it was naturally maintained
in the inverse order that things were added to it (thus default would be at the
end).

(2) Now faces are stored in a hash table whose key is the face and whose value
contains various pieces of data, including the face ID.

(3) This face ID is allocated in increasing order (see e.g. this code in 
xfaces.c:
      Lisp_Object face_id = make_fixnum (next_lface_id);
      lface_id_to_name[next_lface_id] = face;
      Fput (face, Qface, face_id);
      ++next_lface_id;

(4) Thus, `face-list` and `frame-face-alist` sorted the faces by face ID in
order to maintain the old ordering behavior. However, the author accidentally
inverted the comparison when doing so.


> Gregory, any counter-arguments?