From 6a49dabdf66198ebbc297fe0097551f2022e6108 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 13 Feb 2024 09:54:51 -0800 Subject: [PATCH] Remove SYMBOL_WITH_POS_{POS,SYM} * src/fns.c (internal_equal): Turn comment into eassert that !symbols_with_pos_enabled. (sxhash_obj): Simplify case of symbol with pos (when enabled). * src/lisp.h (XSYMBOL_WITH_POS_SYM, XSYMBOL_WITH_POS_POS) (maybe_remove_pos_from_symbol): New inline functions. (SYMBOL_WITH_POS_SYM, SYMBOL_WITH_POS_POS): Remove. All uses replaced by the new functions. This avoids some double-checking in the source code, simplifies the code overall, and avoids the need for "Type checking is done in the following macro" comments to explain unusual code. (cherry picked from commit 10c6aea4434b1c9ccea30a1f87f301ab2c9bade6) --- src/data.c | 16 +++++++--------- src/fns.c | 44 +++++++++++++++++++++----------------------- src/lisp.h | 43 ++++++++++++++++++++++++------------------- src/lread.c | 3 +-- src/timefns.c | 6 ++---- 5 files changed, 55 insertions(+), 57 deletions(-) diff --git a/src/data.c b/src/data.c index 0c47750cb75..530bb774171 100644 --- a/src/data.c +++ b/src/data.c @@ -791,18 +791,16 @@ DEFUN ("bare-symbol", Fbare_symbol, Sbare_symbol, 1, 1, 0, doc: /* Extract, if need be, the bare symbol from SYM, a symbol. */) (register Lisp_Object sym) { - if (BARE_SYMBOL_P (sym)) - return sym; - /* Type checking is done in the following macro. */ - return SYMBOL_WITH_POS_SYM (sym); + CHECK_SYMBOL (sym); + return BARE_SYMBOL_P (sym) ? sym : XSYMBOL_WITH_POS_SYM (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) { - /* Type checking is done in the following macro. */ - return SYMBOL_WITH_POS_POS (ls); + CHECK_TYPE (SYMBOL_WITH_POS_P (ls), Qsymbol_with_pos_p, ls); + return XSYMBOL_WITH_POS_POS (ls); } DEFUN ("remove-pos-from-symbol", Fremove_pos_from_symbol, @@ -812,7 +810,7 @@ Otherwise, return ARG unchanged. Compare with `bare-symbol'. */) (register Lisp_Object arg) { if (SYMBOL_WITH_POS_P (arg)) - return (SYMBOL_WITH_POS_SYM (arg)); + return XSYMBOL_WITH_POS_SYM (arg); return arg; } @@ -829,14 +827,14 @@ the position will be taken. */) if (BARE_SYMBOL_P (sym)) bare = sym; else if (SYMBOL_WITH_POS_P (sym)) - bare = XSYMBOL_WITH_POS (sym)->sym; + bare = XSYMBOL_WITH_POS_SYM (sym); else wrong_type_argument (Qsymbolp, sym); if (FIXNUMP (pos)) position = pos; else if (SYMBOL_WITH_POS_P (pos)) - position = XSYMBOL_WITH_POS (pos)->pos; + position = XSYMBOL_WITH_POS_POS (pos); else wrong_type_argument (Qfixnum_or_symbol_with_pos_p, pos); diff --git a/src/fns.c b/src/fns.c index 61d87752777..918ba0370e8 100644 --- a/src/fns.c +++ b/src/fns.c @@ -2782,13 +2782,8 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind, /* A symbol with position compares the contained symbol, and is `equal' to the corresponding ordinary symbol. */ - if (symbols_with_pos_enabled) - { - if (SYMBOL_WITH_POS_P (o1)) - o1 = SYMBOL_WITH_POS_SYM (o1); - if (SYMBOL_WITH_POS_P (o2)) - o2 = SYMBOL_WITH_POS_SYM (o2); - } + o1 = maybe_remove_pos_from_symbol (o1); + o2 = maybe_remove_pos_from_symbol (o2); if (BASE_EQ (o1, o2)) return true; @@ -2869,11 +2864,14 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind, if (TS_NODEP (o1)) return treesit_node_eq (o1, o2); #endif - if (SYMBOL_WITH_POS_P(o1)) /* symbols_with_pos_enabled is false. */ - return (BASE_EQ (XSYMBOL_WITH_POS (o1)->sym, - XSYMBOL_WITH_POS (o2)->sym) - && BASE_EQ (XSYMBOL_WITH_POS (o1)->pos, - XSYMBOL_WITH_POS (o2)->pos)); + if (SYMBOL_WITH_POS_P (o1)) + { + eassert (!symbols_with_pos_enabled); + return (BASE_EQ (XSYMBOL_WITH_POS_SYM (o1), + XSYMBOL_WITH_POS_SYM (o2)) + && BASE_EQ (XSYMBOL_WITH_POS_POS (o1), + XSYMBOL_WITH_POS_POS (o2))); + } /* Aside from them, only true vectors, char-tables, compiled functions, and fonts (font-spec, font-entity, font-object) @@ -4465,9 +4463,8 @@ reduce_emacs_uint_to_hash_hash (EMACS_UINT x) static EMACS_INT sxhash_eq (Lisp_Object key) { - if (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (key)) - key = SYMBOL_WITH_POS_SYM (key); - return XHASH (key) ^ XTYPE (key); + Lisp_Object k = maybe_remove_pos_from_symbol (key); + return XHASH (k) ^ XTYPE (k); } static EMACS_INT @@ -5247,12 +5244,15 @@ sxhash_obj (Lisp_Object obj, int depth) hash = sxhash_combine (hash, sxhash_obj (XOVERLAY (obj)->plist, depth)); return hash; } - else if (symbols_with_pos_enabled && 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 - address as hash. */ - return XHASH (obj); + { + if (symbols_with_pos_enabled && pvec_type == PVEC_SYMBOL_WITH_POS) + obj = XSYMBOL_WITH_POS_SYM (obj); + + /* Others are 'equal' if they are 'eq', so take their + address as hash. */ + return XHASH (obj); + } } case Lisp_Cons: @@ -5447,9 +5447,7 @@ usage: (make-hash-table &rest KEYWORD-ARGS) */) /* See if there's a `:test TEST' among the arguments. */ ptrdiff_t i = get_key_arg (QCtest, nargs, args, used); - Lisp_Object test = i ? args[i] : Qeql; - if (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (test)) - test = SYMBOL_WITH_POS_SYM (test); + Lisp_Object test = i ? maybe_remove_pos_from_symbol (args[i]) : Qeql; const struct hash_table_test *testdesc; if (BASE_EQ (test, Qeq)) testdesc = &hashtest_eq; diff --git a/src/lisp.h b/src/lisp.h index 796c7867b4c..e9b0bd522af 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -1113,6 +1113,27 @@ XSYMBOL_WITH_POS (Lisp_Object a) return XUNTAG (a, Lisp_Vectorlike, struct Lisp_Symbol_With_Pos); } +INLINE Lisp_Object +XSYMBOL_WITH_POS_SYM (Lisp_Object a) +{ + Lisp_Object sym = XSYMBOL_WITH_POS (a)->sym; + eassert (BARE_SYMBOL_P (sym)); + return sym; +} + +INLINE Lisp_Object +XSYMBOL_WITH_POS_POS (Lisp_Object a) +{ + return XSYMBOL_WITH_POS (a)->pos; +} + +INLINE Lisp_Object +maybe_remove_pos_from_symbol (Lisp_Object x) +{ + return (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (x) + ? XSYMBOL_WITH_POS_SYM (x) : x); +} + INLINE struct Lisp_Symbol * ATTRIBUTE_NO_SANITIZE_UNDEFINED XBARE_SYMBOL (Lisp_Object a) { @@ -1128,7 +1149,7 @@ XSYMBOL (Lisp_Object a) if (!BARE_SYMBOL_P (a)) { eassert (symbols_with_pos_enabled); - a = XSYMBOL_WITH_POS (a)->sym; + a = XSYMBOL_WITH_POS_SYM (a); } return XBARE_SYMBOL (a); } @@ -1322,9 +1343,9 @@ INLINE bool EQ (Lisp_Object x, Lisp_Object y) { return BASE_EQ ((symbols_with_pos_enabled && SYMBOL_WITH_POS_P (x) - ? XSYMBOL_WITH_POS (x)->sym : x), + ? XSYMBOL_WITH_POS_SYM (x) : x), (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (y) - ? XSYMBOL_WITH_POS (y)->sym : y)); + ? XSYMBOL_WITH_POS_SYM (y) : y)); } INLINE intmax_t @@ -2809,22 +2830,6 @@ XOVERLAY (Lisp_Object a) return XUNTAG (a, Lisp_Vectorlike, struct Lisp_Overlay); } -INLINE Lisp_Object -SYMBOL_WITH_POS_SYM (Lisp_Object a) -{ - if (!SYMBOL_WITH_POS_P (a)) - wrong_type_argument (Qsymbol_with_pos_p, a); - return XSYMBOL_WITH_POS (a)->sym; -} - -INLINE Lisp_Object -SYMBOL_WITH_POS_POS (Lisp_Object a) -{ - if (!SYMBOL_WITH_POS_P (a)) - wrong_type_argument (Qsymbol_with_pos_p, a); - return XSYMBOL_WITH_POS (a)->pos; -} - INLINE bool USER_PTRP (Lisp_Object x) { diff --git a/src/lread.c b/src/lread.c index 551bfd735a2..c11c641440d 100644 --- a/src/lread.c +++ b/src/lread.c @@ -5063,8 +5063,7 @@ it defaults to the value of `obarray'. */) { /* If already a symbol, we don't do shorthand-longhand translation, as promised in the docstring. */ - Lisp_Object sym = (symbols_with_pos_enabled && SYMBOL_WITH_POS_P (name) - ? XSYMBOL_WITH_POS (name)->sym : name); + Lisp_Object sym = maybe_remove_pos_from_symbol (name); string = XSYMBOL (name)->u.s.name; tem = oblookup (obarray, SSDATA (string), SCHARS (string), SBYTES (string)); diff --git a/src/timefns.c b/src/timefns.c index fc1edf136cb..0ecbb6e6793 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -1765,10 +1765,8 @@ but new code should not rely on it. */) well, since we accept it as input? */ struct lisp_time t; 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); + form = (!NILP (form) ? maybe_remove_pos_from_symbol (form) + : current_time_list ? Qlist : Qt); if (BASE_EQ (form, Qlist)) return ticks_hz_list4 (t.ticks, t.hz); if (BASE_EQ (form, Qinteger)) -- 2.39.5