bug#65051: internal_equal manipulates symbols with position without checking symbols-with-pos-enabled.

[Stefan Monnier]

>> >> Could you explain why you think it's a bug?
>> > When symbols-with-pos-enabled is non-nil, the two arguments to that
>> > equal call are equal.  That is the point of s-w-p-e.
>> AFAIK the point of the `symbols-with-pos-enabled` is to try and keep the
>> performance impact of sympos under control, and that matters only for
>> `eq`, so I don't think there's a strong reason here for `equal` to pay
>> attention to it.
> Which is like saying you're happy for it to be undefined.

Not quite.  I'm saying that as far as technical reasons go, I can't see
any reason why `equal` needs to pay attention to
`symbols-with-pos-enabled`.  IOW affecting the behavior of `equal` is
*not* part of "the point of s-w-p-e".

> In the code at the moment, the result of `equal' on symbols with
> position is undefined, i.e. it returns a random value.

In which sense?
AFAICT it returns non-nil iff the underlying bare symbols are `eq`.
That does not sound "random" at all to me.
What am I missing?

>> So I'm still wondering why you think it's a bug.
> Because it violates the definition and basic understanding of equal.

Could you expand on that, e.g. explaining which part of your
understanding of "the definition and basic understanding of equal"
it violates?

> It's a special case when no special case is needed.

Making `equal` depend on a global variable is also introducing
a special case.  IOW, all choices suck in one way or another.
I think we need more practical and concrete reasons to prefer one over
another.  Philosophical arguments seem rather weak here.

>> AFAICT whether sympos should be `equal` to others and/or to bare symbols
>> is something we pretty much get to choose freely based on convenience:
> No we don't.  They have to be chosen to be as consistent as possible
> with the rest of Emacs.

`equal` is not self-consistent.  It compares hash-tables like `eq` but
looks inside vectors.  It ignores strings' properties.  The list goes on
and on.

>> either the current behavior or the one you now advocate are perfectly
>> acceptable and not bugs.
> The current behaviour is a bug.

Hmm... This subthread is supposed to answer my question about why you
think it's a bug.  So just re-stating it is not very helpful.
Please try and articulate more precisely *why* you think it's the case.
Is it a gut-feeling?

> It was me that coded up that amendment to equal, and I can remember
> simply not taking into account the scenario we're talking about.

Which scenario?

>> As I said elsewhere, I'm not sure which choice is best, but at least we
>> have some experience with the current choice ....
> I rather doubt that.  When have SWPs, when symbols-with-pos-enabled is
> nil, been tested by equal, apart from in tests, maybe?

We don't know, admittedly, but we do know that if/when it has happened,
it hasn't caused any problem so far.

>> .... and I haven't seen any clear problem with it yet, so I'd tend to
>> lean towards keeping the current behavior.
> I'm wondering why you're making such a big thing out of it.  It's a
> small change which will increase consistency and predictability in
> Emacs in a small way, without any negative effects.

I don't see either of the two options as being more consistent or
more predictable.  You can see symbols' positions as being similar to
strings' properties, which `equal` gleefully ignores.

I think the main reasons I'm rather opposed are:

- I don't like making `equal` depend on a global variable.  It makes it
  impure, and will invalidate existing optimizations, exactly like we've
  just witnessed for `eq`.
- I consider `symbols-with-pos-enabled` to be a wart, so I'd rather try
  and minimize its use as much as possible.

>> What would be the concrete advantages of the new behavior compared to
>> the current one?
> There are no "concrete" advantages, aside from an insignificant increase
> in speed for Emacs when not byte compiling.  The code and the
> documentation currently don't match.  Fixing the code, by removing a
> special case, is easier and more satisfying than documenting that
> special case in the Elisp manual.

Then, I'd vote to fix the doc rather than the code.


        Stefan