bug#69684: Functionality of Fbare_symbol has been lost.

bug#69684: Functionality of Fbare_symbol has been lost.

[Alan Mackenzie]

Hello Paul, hello Emacs.

Since a recent commit, Fbare_symbol (in src/data.c) no longer works on a
symbol with position when symbols-with-pos-enabled is nil, instead
signalling an error.

This is due to the new CHECK_SYMBOL (sym); statement in Fbare_symbol,
which wasn't there before.

Since I merged master into my development branch, I can no longer build
it because of this change.  A rapid reversion to the previous
functionality would be appreciated.  :-)

Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#69684: Functionality of Fbare_symbol has been lost.

[Eli Zaretskii]

> Cc: acm@muc.de, Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 9 Mar 2024 23:23:50 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> Hello Paul, hello Emacs.
> 
> Since a recent commit, Fbare_symbol (in src/data.c) no longer works on a
> symbol with position when symbols-with-pos-enabled is nil, instead
> signalling an error.
> 
> This is due to the new CHECK_SYMBOL (sym); statement in Fbare_symbol,
> which wasn't there before.
> 
> Since I merged master into my development branch, I can no longer build
> it because of this change.  A rapid reversion to the previous
> functionality would be appreciated.  :-)

Couldn't you fix this on your branch by (what sounds like a trivial)
change in Fbare_symbol?

Btw, why is your branch special in this regard?

bug#69684: Functionality of Fbare_symbol has been lost.

[Alan Mackenzie]

Hello, Eli.

On Sun, Mar 10, 2024 at 07:57:26 +0200, Eli Zaretskii wrote:
> > Cc: acm@muc.de, Paul Eggert <eggert@cs.ucla.edu>
> > Date: Sat, 9 Mar 2024 23:23:50 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > Hello Paul, hello Emacs.

> > Since a recent commit, Fbare_symbol (in src/data.c) no longer works on a
> > symbol with position when symbols-with-pos-enabled is nil, instead
> > signalling an error.

> > This is due to the new CHECK_SYMBOL (sym); statement in Fbare_symbol,
> > which wasn't there before.

> > Since I merged master into my development branch, I can no longer build
> > it because of this change.  A rapid reversion to the previous
> > functionality would be appreciated.  :-)

> Couldn't you fix this on your branch by (what sounds like a trivial)
> change in Fbare_symbol?

I've done that already here, of course.  But I was wanting to make a
commit yesterday evening, which of course I can't with this bug
unresolved.  Paul is quite particular over the exact formulation of these
macros and inline functions, which have an important influence on Emacs's
speed.  So I thought I would give him a chance to fix it neatly first.

> Btw, why is your branch special in this regard?

It's the branch where I'm implementing position information in doc
strings, using symbols with position to get this information.  This is
bug #67455.  Progress is pretty much at the stage where we can start
discussing the exact presentation of the information in backtraces, etc.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#69684: Functionality of Fbare_symbol has been lost.

[Eli Zaretskii]

> Date: Sun, 10 Mar 2024 09:21:15 +0000
> Cc: 69684@debbugs.gnu.org, eggert@cs.ucla.edu
> From: Alan Mackenzie <acm@muc.de>
> 
> > > Since I merged master into my development branch, I can no longer build
> > > it because of this change.  A rapid reversion to the previous
> > > functionality would be appreciated.  :-)
> 
> > Couldn't you fix this on your branch by (what sounds like a trivial)
> > change in Fbare_symbol?
> 
> I've done that already here, of course.  But I was wanting to make a
> commit yesterday evening, which of course I can't with this bug
> unresolved.  Paul is quite particular over the exact formulation of these
> macros and inline functions, which have an important influence on Emacs's
> speed.  So I thought I would give him a chance to fix it neatly first.

OK, so let's wait for Paul to chime in.

bug#69684: Functionality of Fbare_symbol has been lost.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On 2024-03-10 03:39, Eli Zaretskii wrote:
> OK, so let's wait for Paul to chime in.

The problem was that I mistakenly believed the documentation when it 
said that a symbol with position behaves like its bare symbol when 
symbols-with-position-enabled is t. Unfortunately it appears that this 
part of the doc wasn't intended to apply to bare-symbol, so when I fixed 
something else involving bare-symbol I got the semantics wrong.

As penance I installed the attached, which makes a simple code change 
along the lines that you suggested and adds a regression test to help 
prevent this bug from happening again.

The hardest part of writing this patch was adjusting the documentation 
to match what I think was the intent of the behavior. Alan, if you find 
mistakes in that please let me know.

A couple of other things.

Currently (position-symbol 'x -1) creates a symbol with position where 
the position is negative; is that intended? The documentation says 
positions are nonnegative.

Also, more test cases of the symbols with position primitives would not 
go amiss. I'm not a good person to write them, though, as I easily get 
confused by symbols with position.

[-- Attachment #2: 0001-Change-bare-symbol-back-to-match-intent.patch --]
[-- Type: text/x-patch, Size: 11689 bytes --]

From 2d61ebb505977af4f9fd90f92a776599a73f8501 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 11 Mar 2024 00:03:39 -0700
Subject: [PATCH] Change bare-symbol back to match intent

Also, attempt to document the intent better.
Problem reported by Alan Mackenzie (Bug#69684).
* src/data.c (Fbare_symbol): Do not signal if the SYM is a symbol
with position and symbols-with-pos-enabled is nil.  Instead,
ignore symbols-with-pos-enabled, as that was the intent.
* test/src/data-tests.el (data-tests-bare-symbol):
New test, to help prevent this bug from reoccurring.
---
 doc/lispref/objects.texi |  6 ++--
 doc/lispref/symbols.texi | 78 ++++++++++++++++++++++------------------
 src/data.c               | 35 +++++++++++-------
 test/src/data-tests.el   |  5 +++
 4 files changed, 73 insertions(+), 51 deletions(-)

diff --git a/doc/lispref/objects.texi b/doc/lispref/objects.texi
index 41171bcaafc..279f449a994 100644
--- a/doc/lispref/objects.texi
+++ b/doc/lispref/objects.texi
@@ -2399,10 +2399,10 @@ Equality Predicates
 if they are integers, strings, markers, vectors, bool-vectors,
 byte-code function objects, char-tables, records, or font objects.
 
-If @var{object1} or @var{object2} is a symbol with position,
-@code{equal} regards it as its bare symbol when
+If @var{object1} or @var{object2} contains symbols with position,
+@code{equal} treats them as if they were their bare symbols when
 @code{symbols-with-pos-enabled} is non-@code{nil}.  Otherwise
-@code{equal} compares two symbols with position by recursively
+@code{equal} compares two symbols with position by
 comparing their components.  @xref{Symbols with Position}.
 
 Other objects are considered @code{equal} only if they are @code{eq}.
diff --git a/doc/lispref/symbols.texi b/doc/lispref/symbols.texi
index 6f9b1ef0ec7..c76bf3d3820 100644
--- a/doc/lispref/symbols.texi
+++ b/doc/lispref/symbols.texi
@@ -780,13 +780,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}.  Symbols
-with position don't themselves have entries in the obarray (though
-their bare symbols do; @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
+A @dfn{symbol with position} is a symbol, called the @dfn{bare symbol},
+together with a nonnegative fixnum called the @dfn{position}.
+Even though a symbol with position often acts like its bare symbol,
+it is not a symbol: instead, it is an object that has both a bare symbol
+and a position.  Because symbols with position are not symbols,
+they don't have entries in the obarray, though their bare symbols
+typically do (@pxref{Creating Symbols}).
+
+The byte compiler uses symbols with position,
+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}.
@@ -799,22 +802,19 @@ Symbols with Position
 operation.  The byte compiler does this before writing its output to
 the compiled Lisp file.
 
-For most purposes, when the flag variable
-@code{symbols-with-pos-enabled} is non-@code{nil}, symbols with
-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 the flag variable @code{symbols-with-pos-enabled} is non-@code{nil},
+a symbol with position ordinarily behaves like its bare symbol.
+For example, @samp{(eq (position-symbol 'foo 12345) 'foo)} yields @code{t},
+and @code{equal} likewise treats a symbol with position as its bare symbol.
 
-When @code{symbols-with-pos-enabled} is @code{nil}, any symbols with
-position continue to exist, but do not behave as symbols, or have the
-other useful properties outlined in the previous paragraph.  @code{eq}
-returns @code{t} when given identical arguments, and @code{equal}
-returns @code{t} when given arguments with @code{equal} components.
+When @code{symbols-with-pos-enabled} is @code{nil}, symbols with
+position behave as themselves, not as symbols.  For example, @samp{(eq
+(position-symbol 'foo 12345) 'foo)} yields @code{nil}, and @code{equal}
+likewise treats a symbol with position as not equal to its bare symbol.
 
 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.
+@code{t} when they run and Emacs runs a little more slowly in this case.
 
 Typically, symbols with position are created by the byte compiler
 calling the reader function @code{read-positioning-symbols}
@@ -822,36 +822,44 @@ Symbols with Position
 @code{position-symbol}.
 
 @defvar symbols-with-pos-enabled
-When this variable is non-@code{nil}, a symbol with position behaves
-like the contained bare symbol.  Emacs runs a little more slowly in
-this case.
+This variable affects the behavior of symbols with position when they
+are not being printed and are not arguments to one of the functions
+defined later in this section.  When this variable is non-@code{nil},
+such a symbol with position behaves like its bare symbol; otherwise it
+behaves as itself, not as a symbol.
 @end defvar
 
 @defvar print-symbols-bare
 When bound to non-@code{nil}, the Lisp printer prints only the bare
 symbol of a symbol with position, ignoring the position.
+Otherwise a symbol with position prints as itself, not as a symbol.
 @end defvar
 
-@defun symbol-with-pos-p symbol
-This function returns @code{t} if @var{symbol} is a symbol with
+@defun symbol-with-pos-p object
+This function returns @code{t} if @var{object} is a symbol with
 position, @code{nil} otherwise.
+Unlike @code{symbolp}, this function ignores @code{symbols-with-pos-enabled}.
 @end defun
 
-@defun bare-symbol symbol
-This function returns the bare symbol contained in @var{symbol}, or
-@var{symbol} itself if it is already a bare symbol.  For any other
-type of object, it signals an error.
+@defun bare-symbol sym
+This function returns the bare symbol of the symbol with
+position @var{sym}, or @var{sym} itself if it is already a symbol.
+For any other type of object, it signals an error.
+This function ignores @code{symbols-with-pos-enabled}.
 @end defun
 
-@defun symbol-with-pos-pos symbol
-This function returns the position, a number, from a symbol with
-position.  For any other type of object, it signals an error.
+@defun symbol-with-pos-pos sympos
+This function returns the position, a nonnegative fixnum, from the symbol with
+position @var{sympos}.  For any other type of object, it signals an error.
+This function ignores @code{symbols-with-pos-enabled}.
 @end defun
 
 @defun position-symbol sym pos
-Make a new symbol with position.  @var{sym} is either a bare symbol or
-a symbol with position, and supplies the symbol part of the new
-object.  @var{pos} is either an integer which becomes the number part
-of the new object, or a symbol with position whose position is used.
+Make a new symbol with position.  The new object's bare symbol is taken
+from @var{sym}, which is either a symbol, or a symbol with position
+whose bare symbol is used.  The new object's position is taken from
+@var{pos}, which is either a nonnegative fixnum, or a symbol with
+position whose position is used.
 Emacs signals an error if either argument is invalid.
+This function ignores @code{symbols-with-pos-enabled}.
 @end defun
diff --git a/src/data.c b/src/data.c
index df08eaf8102..35f4c82c68f 100644
--- a/src/data.c
+++ b/src/data.c
@@ -339,7 +339,8 @@ DEFUN ("bare-symbol-p", Fbare_symbol_p, Sbare_symbol_p, 1, 1, 0,
 }
 
 DEFUN ("symbol-with-pos-p", Fsymbol_with_pos_p, Ssymbol_with_pos_p, 1, 1, 0,
-       doc: /* Return t if OBJECT is a symbol together with position.  */
+       doc: /* Return t if OBJECT is a symbol together with position.
+Ignore `symbols-with-pos-enabled'.  */
        attributes: const)
   (Lisp_Object object)
 {
@@ -789,25 +790,32 @@ DEFUN ("symbol-name", Fsymbol_name, Ssymbol_name, 1, 1, 0,
 }
 
 DEFUN ("bare-symbol", Fbare_symbol, Sbare_symbol, 1, 1, 0,
-       doc: /* Extract, if need be, the bare symbol from SYM, a symbol.  */)
+       doc: /* Extract, if need be, the bare symbol from SYM.
+SYM is either a symbol or a symbol with position.
+Ignore `symbols-with-pos-enabled'.  */)
   (register Lisp_Object sym)
 {
-  CHECK_SYMBOL (sym);
-  return BARE_SYMBOL_P (sym) ? sym : XSYMBOL_WITH_POS_SYM (sym);
+  if (BARE_SYMBOL_P (sym))
+    return sym;
+  if (SYMBOL_WITH_POS_P (sym))
+    return XSYMBOL_WITH_POS_SYM (sym);
+  xsignal2 (Qwrong_type_argument, list2 (Qsymbolp, Qsymbol_with_pos_p), sym);
 }
 
 DEFUN ("symbol-with-pos-pos", Fsymbol_with_pos_pos, Ssymbol_with_pos_pos, 1, 1, 0,
-       doc: /* Extract the position from a symbol with position.  */)
-  (register Lisp_Object ls)
+       doc: /* Extract the position from the symbol with position SYMPOS.
+Ignore `symbols-with-pos-enabled'.  */)
+  (register Lisp_Object sympos)
 {
-  CHECK_TYPE (SYMBOL_WITH_POS_P (ls), Qsymbol_with_pos_p, ls);
-  return XSYMBOL_WITH_POS_POS (ls);
+  CHECK_TYPE (SYMBOL_WITH_POS_P (sympos), Qsymbol_with_pos_p, sympos);
+  return XSYMBOL_WITH_POS_POS (sympos);
 }
 
 DEFUN ("remove-pos-from-symbol", Fremove_pos_from_symbol,
        Sremove_pos_from_symbol, 1, 1, 0,
        doc: /* If ARG is a symbol with position, return it without the position.
-Otherwise, return ARG unchanged.  Compare with `bare-symbol'.  */)
+Otherwise, return ARG unchanged.  Ignore `symbols-with-pos-enabled'.
+Compare with `bare-symbol'.  */)
   (register Lisp_Object arg)
 {
   if (SYMBOL_WITH_POS_P (arg))
@@ -816,10 +824,11 @@ DEFUN ("remove-pos-from-symbol", Fremove_pos_from_symbol,
 }
 
 DEFUN ("position-symbol", Fposition_symbol, Sposition_symbol, 2, 2, 0,
-       doc: /* Create a new symbol with position.
+       doc: /* Make a new symbol with position.
 SYM is a symbol, with or without position, the symbol to position.
-POS, the position, is either a fixnum or a symbol with position from which
-the position will be taken.  */)
+POS, the position, is either a nonnegative fixnum,
+or a symbol with position from which the position will be taken.
+Ignore `symbols-with-pos-enabled'.  */)
      (register Lisp_Object sym, register Lisp_Object pos)
 {
   Lisp_Object bare = Fbare_symbol (sym);
@@ -4374,7 +4383,7 @@ #define PUT_ERROR(sym, tail, msg)			\
 
   DEFSYM (Qsymbols_with_pos_enabled, "symbols-with-pos-enabled");
   DEFVAR_BOOL ("symbols-with-pos-enabled", symbols_with_pos_enabled,
-               doc: /* Non-nil when "symbols with position" can be used as symbols.
+               doc: /* If non-nil, a symbol with position ordinarily behaves as its bare symbol.
 Bind this to non-nil in applications such as the byte compiler.  */);
   symbols_with_pos_enabled = false;
 
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 8af7e902109..ad3b2071254 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -833,4 +833,9 @@ data-tests-defalias
   (should-error (defalias 'data-tests--da-c 'data-tests--da-d)
                 :type 'cyclic-function-indirection))
 
+(ert-deftest data-tests-bare-symbol ()
+  (dolist (symbols-with-pos-enabled '(nil t))
+    (dolist (sym (list nil t 'xyzzy (make-symbol "")))
+      (should (eq sym (bare-symbol (position-symbol sym 0)))))))
+
 ;;; data-tests.el ends here
-- 
2.40.1

bug#69684: Functionality of Fbare_symbol has been lost.

[Alan Mackenzie]

Hello Paul.

On Mon, Mar 11, 2024 at 00:38:39 -0700, Paul Eggert wrote:
> On 2024-03-10 03:39, Eli Zaretskii wrote:
> > OK, so let's wait for Paul to chime in.

> The problem was that I mistakenly believed the documentation when it 
> said that a symbol with position behaves like its bare symbol when 
> symbols-with-position-enabled is t. Unfortunately it appears that this 
> part of the doc wasn't intended to apply to bare-symbol, so when I fixed 
> something else involving bare-symbol I got the semantics wrong.

I think the doc and doc strings were somewhat unclear when it came to
the less obvious cases.

> As penance I installed the attached, which makes a simple code change 
> along the lines that you suggested and adds a regression test to help 
> prevent this bug from happening again.

Thanks!

> The hardest part of writing this patch was adjusting the documentation 
> to match what I think was the intent of the behavior. Alan, if you find 
> mistakes in that please let me know.

That might take a little tine.  I've broken my right arm, which makes
doing Emacs strenuous work.  But I'm sure the patch  will work.

It's also worth pointing out that the uses of SWPs are expanding (see
bug #67455), something I've got mixed feelings about, even though I'm
doing the implementation.

> A couple of other things.

> Currently (position-symbol 'x -1) creates a symbol with position where 
> the position is negative; is that intended? The documentation says 
> positions are nonnegative.

Yes.  I think that back when it was being done, there was no convenient
way to exclude -ve numbers.

> Also, more test cases of the symbols with position primitives would not 
> go amiss. I'm not a good person to write them, though, as I easily get 
> confused by symbols with position.

Yes, I agree here, too.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany)