Re: MPS: Forwording symbols

[Helmut Eller <eller.helmut@gmail.com>]

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

On Mon, Jun 24 2024, Gerd Möllmann wrote:

>> Here is an attempt to use an enum.  I'm not sure that it's easier to
>> read and the enum constants have odd names.
>
> Pushed!

I have one more patch: it moves the bufoffset field back to the union.
I think that's a bit nicer; and with the predicate enum there is enough
room to do it.

There other patch, sets some fields in a popped handler to nil to avoid
a potential leak.

> We even spared a word in the struct :-).

Right, I haven't even noticed that.  Now the union only needs one word
and the type fields could be packed in 3 bits.  That would probably be
small enough to store it in a symbol.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Move-the-Lisp_Fwd.bufoffset-field-back-to-the-union.patch --]
[-- Type: text/x-diff, Size: 2947 bytes --]

From 770aaffe0ae52fb9305f9e2b646803ab2c4c0f84 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Mon, 24 Jun 2024 10:06:54 +0200
Subject: [PATCH 1/2] Move the Lisp_Fwd.bufoffset field back to the union

* src/lisp.h (struct Lisp_Fwd): With the predicate enum, we can now pack
the offset and the predicate into a one-word struct.
(XBUFFER_OFFSET): Use the new field name.
* src/buffer.c (DEFVAR_PER_BUFFER): Create the one-word struct.
* src/data.c (store_symval_forwarding): Use the new field name.
---
 src/buffer.c | 15 +++++++++------
 src/data.c   |  2 +-
 src/lisp.h   |  9 ++++++---
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 39b5e29067a..a69a9a3bbd8 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -5017,14 +5017,17 @@ init_buffer (void)
    that nil is allowed too).  DOC is a dummy where you write the doc
    string as a comment.  */
 
-#define DEFVAR_PER_BUFFER(lname, vname, predicate, doc)		\
+#define DEFVAR_PER_BUFFER(lname, vname, predicate_, doc)	\
   do {								\
-    static const struct Lisp_Fwd bo_fwd				\
-	= { .type = Lisp_Fwd_Buffer_Obj,			\
-	    .bufoffset = offsetof (struct buffer, vname##_),	\
-	    .u.bufpredicate = FWDPRED_##predicate };		\
+    static const struct Lisp_Fwd bo_fwd = {			\
+      .type = Lisp_Fwd_Buffer_Obj,				\
+      .u.buf = {						\
+	.offset = offsetof (struct buffer, vname##_),		\
+	.predicate = FWDPRED_##predicate_			\
+      }								\
+    };								\
     static_assert (offsetof (struct buffer, vname##_)		\
-		   < (1 << 8 * sizeof bo_fwd.bufoffset));	\
+		   < (1 << 8 * sizeof bo_fwd.u.buf.offset));	\
     defvar_per_buffer (&bo_fwd, lname);				\
   } while (0)
 
diff --git a/src/data.c b/src/data.c
index 9fd439e07b5..dcf869c1a0e 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1513,7 +1513,7 @@ store_symval_forwarding (lispfwd valcontents, Lisp_Object newval,
       {
 	int offset = XBUFFER_OFFSET (valcontents);
 	if (!NILP (newval))
-	  check_fwd_predicate (valcontents->u.bufpredicate, newval);
+	  check_fwd_predicate (valcontents->u.buf.predicate, newval);
 	if (buf == NULL)
 	  buf = current_buffer;
 	set_per_buffer_value (buf, offset, newval);
diff --git a/src/lisp.h b/src/lisp.h
index e3c2dce7df0..912b208a213 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3180,13 +3180,16 @@ #define INT_TO_INTEGER(expr) \
 struct Lisp_Fwd
 {
   enum Lisp_Fwd_Type type : 8;
-  uint16_t bufoffset;	       /* used if type == Lisp_Fwd_Buffer_Obj */
   union
   {
     intmax_t *intvar;
     bool *boolvar;
     Lisp_Object *objvar;
-    enum Lisp_Fwd_Predicate bufpredicate;
+    struct
+    {
+      uint16_t offset;
+      enum Lisp_Fwd_Predicate predicate : 8;
+    } buf;
     int kbdoffset;
   } u;
 };
@@ -3207,7 +3210,7 @@ BUFFER_OBJFWDP (lispfwd a)
 XBUFFER_OFFSET (lispfwd a)
 {
   eassert (BUFFER_OBJFWDP (a));
-  return a->bufoffset;
+  return a->u.buf.offset;
 }
 
 INLINE bool
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-In-pop_handler-clear-references-in-the-dropped-handl.patch --]
[-- Type: text/x-diff, Size: 999 bytes --]

From 5b927463be1ee9055ef18ed654307699f5d17fea Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Mon, 24 Jun 2024 16:49:09 +0200
Subject: [PATCH 2/2] In pop_handler, clear references in the dropped handler

* src/eval.c (pop_handler): Set the val and tag_or_ch fields to nil.
Without this, the referenced objects can't be collected until a new
handler is pushed.
---
 src/eval.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/eval.c b/src/eval.c
index 6f008804e10..40c3c6394b0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1281,7 +1281,14 @@ #define clobbered_eassert(E) verify (sizeof (E) != 0)
 void
 pop_handler (void)
 {
-  handlerlist = handlerlist->next;
+  struct handler *old = handlerlist;
+  struct handler *new = old->next;
+  handlerlist = new;
+#ifdef HAVE_MPS
+  eassert (new->nextfree == old);
+  old->val = Qnil;
+  old->tag_or_ch = Qnil;
+#endif
 }
 
 /* Set up a catch, then call C function FUNC on argument ARG.
-- 
2.39.2