unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Alan Mackenzie <acm@muc.de>
Cc: 65051@debbugs.gnu.org
Subject: bug#65051: internal_equal manipulates symbols with position without checking symbols-with-pos-enabled.
Date: Sat, 12 Aug 2023 01:36:08 -0400	[thread overview]
Message-ID: <jwv5y5ku8u2.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <ZNUt7-mqF_admTg5@ACM> (Alan Mackenzie's message of "Thu, 10 Aug 2023 18:35:27 +0000")

> 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))






  reply	other threads:[~2023-08-12  5:36 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-04 14:00 bug#65051: internal_equal manipulates symbols with position without checking symbols-with-pos-enabled Alan Mackenzie
2023-08-04 14:32 ` Eli Zaretskii
2023-08-04 14:59   ` Alan Mackenzie
2023-08-04 15:27     ` Eli Zaretskii
2023-08-04 17:06       ` Alan Mackenzie
2023-08-04 18:01         ` Eli Zaretskii
2023-08-05 10:45           ` Alan Mackenzie
2023-08-05 10:57             ` Eli Zaretskii
2023-08-05 11:52               ` Alan Mackenzie
2023-08-05 12:13                 ` Eli Zaretskii
2023-08-05 13:04                   ` Alan Mackenzie
2023-08-05 13:13                     ` Eli Zaretskii
2023-08-13 16:14                       ` Alan Mackenzie
2023-08-05 14:40 ` Mattias Engdegård
2023-08-05 16:59   ` Alan Mackenzie
2023-08-05 17:02     ` Mattias Engdegård
2023-08-05 21:07   ` Alan Mackenzie
2023-08-06 13:37     ` Mattias Engdegård
2023-08-06 15:02       ` Alan Mackenzie
2023-08-07  8:58         ` Mattias Engdegård
2023-08-07  9:44           ` Alan Mackenzie
2023-08-09 18:45             ` Mattias Engdegård
2023-08-07  3:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-07  9:20   ` Alan Mackenzie
2023-08-08  2:56     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-08 15:33       ` Alan Mackenzie
2023-08-10  3:28         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-10  9:14           ` Alan Mackenzie
2023-08-10 14:28             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-10 18:35               ` Alan Mackenzie
2023-08-12  5:36                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2023-08-12  6:10                   ` Eli Zaretskii
2023-08-12 18:46                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-12 19:10                       ` Eli Zaretskii
2023-08-13 15:27                       ` Alan Mackenzie
2023-08-12 10:41                   ` Alan Mackenzie
2023-08-12 18:07                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-13 13:52                       ` Alan Mackenzie
2023-08-12 21:59                   ` Alan Mackenzie
2023-08-11  0:51         ` Dmitry Gutov
2023-08-11 10:42           ` Alan Mackenzie
2023-08-11 11:18             ` Dmitry Gutov
2023-08-11 12:05               ` Alan Mackenzie
2023-08-11 13:19                 ` Dmitry Gutov
2023-08-11 14:04                   ` Alan Mackenzie
2023-08-11 18:15                     ` Dmitry Gutov
     [not found] ` <handler.65051.B.169115764532326.ack@debbugs.gnu.org>
2023-09-04 12:57   ` bug#65051: Acknowledgement (internal_equal manipulates symbols with position without checking symbols-with-pos-enabled.) Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwv5y5ku8u2.fsf-monnier+emacs@gnu.org \
    --to=bug-gnu-emacs@gnu.org \
    --cc=65051@debbugs.gnu.org \
    --cc=acm@muc.de \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).