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

[Stefan Monnier]

> I'm proposing correctness, according to a coherent definition of
> symbols-with-pos-enabled.  I was surprised indeed, and continue to be
> surprised, that you do not see this correction as a correction.  To me,
> it's obvious.

I guess it's because you see it as a feature that (symbol-function
(position-symbol 'a 3)) signals an error when `symbols-with-pos-enabled`
is nil, whereas I see it as a misfeature we should try and fix.

> That is the case, yes.  There are no current use-cases for SWPs with
> s-w-p-enabled nil.

Right.  So all the code which behaves differently when encountering an
SWP depending on the value of `s-w-p-enabled` has only one of the two
branches tested.  My preference for making the behavior oblivious to
`s-w-p-enabled` (except for those rare cases where it's needed for
performance reasons) removes these untested code paths.

In any case, here's my "counter offer" (BTW, why we do handle SWP
specially in `time-convert`?).


        Stefan


diff --git a/doc/lispref/symbols.texi b/doc/lispref/symbols.texi
index 34db0caf3a8..0eb3e8f211d 100644
--- a/doc/lispref/symbols.texi
+++ b/doc/lispref/symbols.texi
@@ -782,11 +782,16 @@ Symbols with Position
 @cindex symbol with position
 
 @cindex bare symbol
-A @dfn{symbol with position} is a symbol, the @dfn{bare symbol},
-together with an unsigned integer called the @dfn{position}.  These
-objects are intended for use by the byte compiler, which records in
-them the position of each symbol occurrence and uses those positions
-in warning and error messages.
+A @dfn{symbol with position} is a pair of a symbol, the @dfn{bare symbol},
+together with an unsigned integer called the @dfn{position}.
+They cannot themselves have entries in obarrays (contrary to their
+bare symbols; @pxref{Creating Symbols}).
+
+Symbols with position are for the use of the byte compiler, which
+records in them the position of each symbol occurrence and uses those
+positions in warning and error messages.  They shouldn't normally be
+used otherwise.  Doing so can cause unexpected results with basic
+Emacs functions such as @code{eq} and @code{equal}.
 
 The printed representation of a symbol with position uses the hash
 notation outlined in @ref{Printed Representation}.  It looks like
@@ -798,11 +803,20 @@ Symbols with Position
 
 For most purposes, when the flag variable
 @code{symbols-with-pos-enabled} is non-@code{nil}, symbols with
-positions behave just as bare symbols do.  For example, @samp{(eq
-#<symbol foo at 12345> foo)} has a value @code{t} when that variable
-is set (but @code{nil} when it isn't set).  Most of the time in Emacs this
-variable is @code{nil}, but the byte compiler binds it to @code{t}
-when it runs.
+positions behave just as their bare symbols would.  For example,
+@samp{(eq #<symbol foo at 12345> foo)} has a value @code{t} when the
+variable is set; likewise, @code{equal} will treat a symbol with
+position argument as its bare symbol.
+
+When @code{symbols-with-pos-enabled} is @code{nil}, any symbols with
+position continue to exist, but do not always behave as symbols.
+Most importantly @code{eq} only returns @code{t} when given truly
+identical arguments, for performance reasons.  @code{equal} on the
+other hand is not affected,
+
+Most of the time in Emacs @code{symbols-with-pos-enabled} is
+@code{nil}, but the byte compiler and the native compiler bind it to
+@code{t} when they run.
 
 Typically, symbols with position are created by the byte compiler
 calling the reader function @code{read-positioning-symbols}
diff --git a/src/fns.c b/src/fns.c
index d7b2e7908b6..5239eb73026 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -5166,7 +5166,7 @@ sxhash_obj (Lisp_Object obj, int depth)
            hash = sxhash_combine (hash, sxhash_obj (XOVERLAY (obj)->plist, 
depth));
            return SXHASH_REDUCE (hash);
          }
-       else if (symbols_with_pos_enabled && pvec_type == PVEC_SYMBOL_WITH_POS)
+       else if (pvec_type == PVEC_SYMBOL_WITH_POS)
          return sxhash_obj (XSYMBOL_WITH_POS (obj)->sym, depth + 1);
        else
          /* Others are 'equal' if they are 'eq', so take their
diff --git a/src/timefns.c b/src/timefns.c
index 151f5b482a3..7e030da3fcd 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -1767,8 +1767,6 @@ DEFUN ("time-convert", Ftime_convert, Stime_convert, 1, 
2, 0,
   enum timeform input_form = decode_lisp_time (time, false, &t, 0);
   if (NILP (form))
     form = current_time_list ? Qlist : Qt;
-  if (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (form))
-    form = SYMBOL_WITH_POS_SYM (form);
   if (BASE_EQ (form, Qlist))
     return ticks_hz_list4 (t.ticks, t.hz);
   if (BASE_EQ (form, Qinteger))