GNU bug report logs - #59137
[PATCH] To minor changes related to overlays

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59137 in the body.
You can then email your comments to 59137 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] To minor changes related to overlays
Date: Tue, 08 Nov 2022 15:14:08 -0800

[Message part 1 (text/plain, inline)]

Tags: patch

X-Debbugs-CC: Stefan Monnier <monnier <at> iro.umontreal.ca>

[0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch (text/x-diff, inline)]

From 023dddaf723aacd6579331f76f61a2741a4e52d5 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 8 Nov 2022 15:00:18 -0800
Subject: [PATCH 1/2] Add itree_empty_p for clarity and reduced coupling

* src/itree.h (itree_empty_p): New predicate.
* src/buffer.h (buffer_has_overlays): Call it.
* src/pdumper.c (dump_buffer): ditto.
* src/alloc.c (mark_buffer): ditto.
---
 src/alloc.c   | 2 +-
 src/buffer.h  | 3 +--
 src/itree.h   | 9 +++++++++
 src/pdumper.c | 2 +-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 6862cf916fb..d815a199fe0 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6548,7 +6548,7 @@ mark_buffer (struct buffer *buffer)
   if (!BUFFER_LIVE_P (buffer))
       mark_object (BVAR (buffer, undo_list));
 
-  if (buffer->overlays)
+  if (!itree_empty_p (buffer->overlays))
     mark_overlays (buffer->overlays->root);
 
   /* If this is an indirect buffer, mark its base buffer.  */
diff --git a/src/buffer.h b/src/buffer.h
index 2e80c8a7b04..08b0420c066 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -1273,8 +1273,7 @@ set_buffer_intervals (struct buffer *b, INTERVAL i)
 INLINE bool
 buffer_has_overlays (void)
 {
-  return current_buffer->overlays
-         && (current_buffer->overlays->root != NULL);
+  return !itree_empty_p (current_buffer->overlays);
 }
 
 /* Functions for accessing a character or byte,
diff --git a/src/itree.h b/src/itree.h
index 10ee0897c37..d6c6fb10591 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -25,6 +25,8 @@ #define ITREE_H
 
 #include "lisp.h"
 
+INLINE_HEADER_BEGIN
+
 /* The tree and node structs are mainly here, so they can be
    allocated.
 
@@ -117,6 +119,11 @@ #define ITREE_H
 				   ptrdiff_t, ptrdiff_t);
 extern struct itree_tree *itree_create (void);
 extern void itree_destroy (struct itree_tree *);
+INLINE bool
+itree_empty_p (struct itree_tree *tree)
+{
+  return !tree || !tree->root;
+}
 extern intmax_t itree_size (struct itree_tree *);
 extern void itree_clear (struct itree_tree *);
 extern void itree_insert (struct itree_tree *, struct itree_node *,
@@ -183,4 +190,6 @@ #define ITREE_FOREACH_ABORT() \
 #define ITREE_FOREACH_NARROW(beg, end) \
   itree_iterator_narrow (itree_iter_, beg, end)
 
+INLINE_HEADER_END
+
 #endif
diff --git a/src/pdumper.c b/src/pdumper.c
index 0a5d96dbb7c..22d3f3f90e4 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2863,7 +2863,7 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
   DUMP_FIELD_COPY (out, buffer, inhibit_buffer_hooks);
   DUMP_FIELD_COPY (out, buffer, long_line_optimizations_p);
 
-  if (buffer->overlays && buffer->overlays->root != NULL)
+  if (!itree_empty_p (buffer->overlays))
     /* We haven't implemented the code to dump overlays.  */
     emacs_abort ();
   else
-- 
2.35.1

[0002-Simplify-ITREE_FOREACH.patch (text/x-diff, inline)]

From 67caf59c7399659a7de273c175134eac88e777ac Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 8 Nov 2022 15:08:00 -0800
Subject: [PATCH 2/2] Simplify ITREE_FOREACH

* src/itree.c (itree_iterator_next): Call itree_iterator_finish if
returning NULL.
* src/itree.h (ITREE_FOREACH): Don't call itree_iterator_finish.
---
 src/itree.c | 3 +++
 src/itree.h | 7 +++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/itree.c b/src/itree.c
index 989173db4e5..74199db3e42 100644
--- a/src/itree.c
+++ b/src/itree.c
@@ -1431,6 +1431,9 @@ itree_iterator_next (struct itree_iterator *g)
 	 after it was pushed: Check if it still intersects. */
     } while (node && ! interval_node_intersects (node, g->begin, g->end));
 
+  if (!node)
+    itree_iterator_finish(g);
+
   return node;
 }
 
diff --git a/src/itree.h b/src/itree.h
index d6c6fb10591..67e258dd832 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -179,10 +179,9 @@ #define ITREE_FOREACH(n, t, beg, end, order)                        \
     { }                                                             \
   else                                                              \
     for (struct itree_iterator *itree_iter_                         \
-            = itree_iterator_start (t, beg, end, ITREE_##order,     \
-                                        __FILE__, __LINE__);        \
-          ((n = itree_iterator_next (itree_iter_))                  \
-           || (itree_iterator_finish (itree_iter_), false));)
+           = itree_iterator_start (t, beg, end, ITREE_##order,      \
+                                   __FILE__, __LINE__);             \
+         (n = itree_iterator_next (itree_iter_));)
 
 #define ITREE_FOREACH_ABORT() \
   itree_iterator_finish (itree_iter_)
-- 
2.35.1

Message #8 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matt Armstrong <matt <at> rfc20.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Thu, 10 Nov 2022 12:38:26 +0200

> From: Matt Armstrong <matt <at> rfc20.org>
> Date: Tue, 08 Nov 2022 15:14:08 -0800
> 
> Tags: patch
> 
> X-Debbugs-CC: Stefan Monnier <monnier <at> iro.umontreal.ca>

Stefan, any comments?

I have just one nit:

> diff --git a/src/itree.c b/src/itree.c
> index 989173db4e5..74199db3e42 100644
> --- a/src/itree.c
> +++ b/src/itree.c
> @@ -1431,6 +1431,9 @@ itree_iterator_next (struct itree_iterator *g)
>  	 after it was pushed: Check if it still intersects. */
>      } while (node && ! interval_node_intersects (node, g->begin, g->end));
>  
> +  if (!node)
> +    itree_iterator_finish(g);
                            ^
Please leave one space between the function's name and the open
parenthesis of the argument list.

Thanks.

Message #11 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Tue, 15 Nov 2022 09:53:32 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> +  if (!node)
>> +    itree_iterator_finish(g);
>                             ^
> Please leave one space between the function's name and the open
> parenthesis of the argument list.

Ahh, yes.  Attached are new patches, rebased to current master, with
that formatting change.

[0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch (text/x-diff, inline)]

From 9cdfe65ee28c68398b9305d15f178358f02f4498 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 8 Nov 2022 15:00:18 -0800
Subject: [PATCH 1/2] Add itree_empty_p for clarity and reduced coupling

* src/itree.h (itree_empty_p): New predicate.
* src/buffer.h (buffer_has_overlays): Call it.
* src/pdumper.c (dump_buffer): ditto.
* src/alloc.c (mark_buffer): ditto.
---
 src/alloc.c   | 2 +-
 src/buffer.h  | 3 +--
 src/itree.h   | 9 +++++++++
 src/pdumper.c | 2 +-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 6862cf916f..d815a199fe 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6548,7 +6548,7 @@ mark_buffer (struct buffer *buffer)
   if (!BUFFER_LIVE_P (buffer))
       mark_object (BVAR (buffer, undo_list));
 
-  if (buffer->overlays)
+  if (!itree_empty_p (buffer->overlays))
     mark_overlays (buffer->overlays->root);
 
   /* If this is an indirect buffer, mark its base buffer.  */
diff --git a/src/buffer.h b/src/buffer.h
index 2e80c8a7b0..08b0420c06 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -1273,8 +1273,7 @@ set_buffer_intervals (struct buffer *b, INTERVAL i)
 INLINE bool
 buffer_has_overlays (void)
 {
-  return current_buffer->overlays
-         && (current_buffer->overlays->root != NULL);
+  return !itree_empty_p (current_buffer->overlays);
 }
 
 /* Functions for accessing a character or byte,
diff --git a/src/itree.h b/src/itree.h
index 10ee0897c3..d6c6fb1059 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -25,6 +25,8 @@ #define ITREE_H
 
 #include "lisp.h"
 
+INLINE_HEADER_BEGIN
+
 /* The tree and node structs are mainly here, so they can be
    allocated.
 
@@ -117,6 +119,11 @@ #define ITREE_H
 				   ptrdiff_t, ptrdiff_t);
 extern struct itree_tree *itree_create (void);
 extern void itree_destroy (struct itree_tree *);
+INLINE bool
+itree_empty_p (struct itree_tree *tree)
+{
+  return !tree || !tree->root;
+}
 extern intmax_t itree_size (struct itree_tree *);
 extern void itree_clear (struct itree_tree *);
 extern void itree_insert (struct itree_tree *, struct itree_node *,
@@ -183,4 +190,6 @@ #define ITREE_FOREACH_ABORT() \
 #define ITREE_FOREACH_NARROW(beg, end) \
   itree_iterator_narrow (itree_iter_, beg, end)
 
+INLINE_HEADER_END
+
 #endif
diff --git a/src/pdumper.c b/src/pdumper.c
index 0a5d96dbb7..22d3f3f90e 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2863,7 +2863,7 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
   DUMP_FIELD_COPY (out, buffer, inhibit_buffer_hooks);
   DUMP_FIELD_COPY (out, buffer, long_line_optimizations_p);
 
-  if (buffer->overlays && buffer->overlays->root != NULL)
+  if (!itree_empty_p (buffer->overlays))
     /* We haven't implemented the code to dump overlays.  */
     emacs_abort ();
   else
-- 
2.35.1

[0002-Simplify-ITREE_FOREACH.patch (text/x-diff, inline)]

From c2fc83ff7e7d49bdb013881453e504f21f038104 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 8 Nov 2022 15:08:00 -0800
Subject: [PATCH 2/2] Simplify ITREE_FOREACH

* src/itree.c (itree_iterator_next): Call itree_iterator_finish if
returning NULL.
* src/itree.h (ITREE_FOREACH): Don't call itree_iterator_finish.
---
 src/itree.c | 3 +++
 src/itree.h | 7 +++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/itree.c b/src/itree.c
index ae69c97d6d..18ee6449ce 100644
--- a/src/itree.c
+++ b/src/itree.c
@@ -1431,6 +1431,9 @@ itree_iterator_next (struct itree_iterator *g)
 	 after it was pushed: Check if it still intersects. */
     } while (node && ! interval_node_intersects (node, g->begin, g->end));
 
+  if (!node)
+    itree_iterator_finish (g);
+
   return node;
 }
 
diff --git a/src/itree.h b/src/itree.h
index d6c6fb1059..67e258dd83 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -179,10 +179,9 @@ #define ITREE_FOREACH(n, t, beg, end, order)                        \
     { }                                                             \
   else                                                              \
     for (struct itree_iterator *itree_iter_                         \
-            = itree_iterator_start (t, beg, end, ITREE_##order,     \
-                                        __FILE__, __LINE__);        \
-          ((n = itree_iterator_next (itree_iter_))                  \
-           || (itree_iterator_finish (itree_iter_), false));)
+           = itree_iterator_start (t, beg, end, ITREE_##order,      \
+                                   __FILE__, __LINE__);             \
+         (n = itree_iterator_next (itree_iter_));)
 
 #define ITREE_FOREACH_ABORT() \
   itree_iterator_finish (itree_iter_)
-- 
2.35.1

Message #14 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Thu, 24 Nov 2022 17:16:06 -0800

Matt Armstrong <matt <at> rfc20.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> +  if (!node)
>>> +    itree_iterator_finish(g);
>>                             ^
>> Please leave one space between the function's name and the open
>> parenthesis of the argument list.
>
> Ahh, yes.  Attached are new patches, rebased to current master, with
> that formatting change.

Should these patches be installed?  Stefan?

Message #17 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Matt Armstrong <matt <at> rfc20.org>, Eli Zaretskii <eliz <at> gnu.org>,
 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Fri, 25 Nov 2022 09:48:25 -0500

> Should these patches be installed?  Stefan?

I don't have an opinion on the `itree_empty_p` patch, but the one about
`itree_iterator_finish` doesn't apply any more because
`itree_iterator_finish` has been removed in the mean time.


        Stefan

Message #20 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Matt Armstrong <matt <at> rfc20.org>, Eli Zaretskii <eliz <at> gnu.org>,
 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Sat, 26 Nov 2022 06:05:36 -0800

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Should these patches be installed?  Stefan?
>
> I don't have an opinion on the `itree_empty_p` patch, but the one about
> `itree_iterator_finish` doesn't apply any more because
> `itree_iterator_finish` has been removed in the mean time.

Matt, if you think these patches are still relevant, could you please
rebase them on top of current master?  Thanks in advance.

Message #23 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Kangas <stefankangas <at> gmail.com>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Sat, 26 Nov 2022 11:37:18 -0800

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>> Should these patches be installed?  Stefan?
>>
>> I don't have an opinion on the `itree_empty_p` patch, but the one about
>> `itree_iterator_finish` doesn't apply any more because
>> `itree_iterator_finish` has been removed in the mean time.
>
> Matt, if you think these patches are still relevant, could you please
> rebase them on top of current master?  Thanks in advance.

Hi Stefan and Stefan,

Attached is the rebased patch for the new helper function (it didn't
change much if at all).  As Stefan suggested, the patch for the iterator
is no longer relevant.

[0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch (text/x-diff, inline)]

From 3e2c4cd143d51c66198dd606e18015eeae42f3ec Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt <at> rfc20.org>
Date: Tue, 8 Nov 2022 15:00:18 -0800
Subject: [PATCH] Add itree_empty_p for clarity and reduced coupling

* src/itree.h (itree_empty_p): New predicate.
* src/buffer.h (buffer_has_overlays): Call it.
* src/pdumper.c (dump_buffer): ditto.
* src/alloc.c (mark_buffer): ditto.
---
 src/alloc.c   | 2 +-
 src/buffer.h  | 3 +--
 src/itree.h   | 9 +++++++++
 src/pdumper.c | 2 +-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 0653f2e0cc..526a25393f 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6553,7 +6553,7 @@ mark_buffer (struct buffer *buffer)
   if (!BUFFER_LIVE_P (buffer))
       mark_object (BVAR (buffer, undo_list));
 
-  if (buffer->overlays)
+  if (!itree_empty_p (buffer->overlays))
     mark_overlays (buffer->overlays->root);
 
   /* If this is an indirect buffer, mark its base buffer.  */
diff --git a/src/buffer.h b/src/buffer.h
index dded0cd98c..9ead875bcf 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -1277,8 +1277,7 @@ set_buffer_intervals (struct buffer *b, INTERVAL i)
 INLINE bool
 buffer_has_overlays (void)
 {
-  return current_buffer->overlays
-         && (current_buffer->overlays->root != NULL);
+  return !itree_empty_p (current_buffer->overlays);
 }
 
 /* Functions for accessing a character or byte,
diff --git a/src/itree.h b/src/itree.h
index 291fa53fd3..248ea9b84d 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -25,6 +25,8 @@ #define ITREE_H
 
 #include "lisp.h"
 
+INLINE_HEADER_BEGIN
+
 /* The tree and node structs are mainly here, so they can be
    allocated.
 
@@ -114,6 +116,11 @@ #define ITREE_H
 				   ptrdiff_t, ptrdiff_t);
 extern struct itree_tree *itree_create (void);
 extern void itree_destroy (struct itree_tree *);
+INLINE bool
+itree_empty_p (struct itree_tree *tree)
+{
+  return !tree || !tree->root;
+}
 extern intmax_t itree_size (struct itree_tree *);
 extern void itree_clear (struct itree_tree *);
 extern void itree_insert (struct itree_tree *, struct itree_node *,
@@ -178,4 +185,6 @@ #define ITREE_FOREACH(n, t, beg, end, order)                        \
 #define ITREE_FOREACH_NARROW(beg, end) \
   itree_iterator_narrow (itree_iter_, beg, end)
 
+INLINE_HEADER_END
+
 #endif
diff --git a/src/pdumper.c b/src/pdumper.c
index fedcd3e404..35e86d2b50 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2863,7 +2863,7 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
   DUMP_FIELD_COPY (out, buffer, inhibit_buffer_hooks);
   DUMP_FIELD_COPY (out, buffer, long_line_optimizations_p);
 
-  if (buffer->overlays && buffer->overlays->root != NULL)
+  if (!itree_empty_p (buffer->overlays))
     /* We haven't implemented the code to dump overlays.  */
     emacs_abort ();
   else
-- 
2.35.1

Message #26 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Sat, 26 Nov 2022 12:07:28 -0800

Matt Armstrong <matt <at> rfc20.org> writes:

> Attached is the rebased patch for the new helper function (it didn't
> change much if at all).  As Stefan suggested, the patch for the iterator
> is no longer relevant.

Thanks.

> From 3e2c4cd143d51c66198dd606e18015eeae42f3ec Mon Sep 17 00:00:00 2001
> From: Matt Armstrong <matt <at> rfc20.org>
> Date: Tue, 8 Nov 2022 15:00:18 -0800
> Subject: [PATCH] Add itree_empty_p for clarity and reduced coupling
>
> * src/itree.h (itree_empty_p): New predicate.
> * src/buffer.h (buffer_has_overlays): Call it.
> * src/pdumper.c (dump_buffer): ditto.
> * src/alloc.c (mark_buffer): ditto.

Equivalently, you can leave out "ditto" so the above is just the below
(I added the bug number too, according to our conventions):

* src/itree.h (itree_empty_p): New predicate.
* src/buffer.h (buffer_has_overlays):
* src/pdumper.c (dump_buffer):
* src/alloc.c (mark_buffer): Call it.  (Bug#59137)

> ---
>  src/alloc.c   | 2 +-
>  src/buffer.h  | 3 +--
>  src/itree.h   | 9 +++++++++
>  src/pdumper.c | 2 +-
>  4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/src/alloc.c b/src/alloc.c
> index 0653f2e0cc..526a25393f 100644
> --- a/src/alloc.c
> +++ b/src/alloc.c
> @@ -6553,7 +6553,7 @@ mark_buffer (struct buffer *buffer)
>    if (!BUFFER_LIVE_P (buffer))
>        mark_object (BVAR (buffer, undo_list));
>
> -  if (buffer->overlays)
> +  if (!itree_empty_p (buffer->overlays))
>      mark_overlays (buffer->overlays->root);

I'm not familiar with this code at all, but I note that the condition
here changes from:

    buffer->overlays

to

    buffer->overlays && buffer->overlays->root

Is that correct?  Unless I missed something, the patch description
doesn't say anything about it.

Message #29 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Kangas <stefankangas <at> gmail.com>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Tue, 29 Nov 2022 15:16:16 -0800

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Matt Armstrong <matt <at> rfc20.org> writes:
>
>> Attached is the rebased patch for the new helper function (it didn't
>> change much if at all).  As Stefan suggested, the patch for the iterator
>> is no longer relevant.
>
> Thanks.
>
>> From 3e2c4cd143d51c66198dd606e18015eeae42f3ec Mon Sep 17 00:00:00 2001
>> From: Matt Armstrong <matt <at> rfc20.org>
>> Date: Tue, 8 Nov 2022 15:00:18 -0800
>> Subject: [PATCH] Add itree_empty_p for clarity and reduced coupling
>>
>> * src/itree.h (itree_empty_p): New predicate.
>> * src/buffer.h (buffer_has_overlays): Call it.
>> * src/pdumper.c (dump_buffer): ditto.
>> * src/alloc.c (mark_buffer): ditto.

Thanks for the tips.


> Equivalently, you can leave out "ditto" so the above is just the below
> (I added the bug number too, according to our conventions):
>
> * src/itree.h (itree_empty_p): New predicate.
> * src/buffer.h (buffer_has_overlays):
> * src/pdumper.c (dump_buffer):
> * src/alloc.c (mark_buffer): Call it.  (Bug#59137)
>
>> ---
>>  src/alloc.c   | 2 +-
>>  src/buffer.h  | 3 +--
>>  src/itree.h   | 9 +++++++++
>>  src/pdumper.c | 2 +-
>>  4 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/alloc.c b/src/alloc.c
>> index 0653f2e0cc..526a25393f 100644
>> --- a/src/alloc.c
>> +++ b/src/alloc.c
>> @@ -6553,7 +6553,7 @@ mark_buffer (struct buffer *buffer)
>>    if (!BUFFER_LIVE_P (buffer))
>>        mark_object (BVAR (buffer, undo_list));
>>
>> -  if (buffer->overlays)
>> +  if (!itree_empty_p (buffer->overlays))
>>      mark_overlays (buffer->overlays->root);
>
> I'm not familiar with this code at all, but I note that the condition
> here changes from:
>
>     buffer->overlays
>
> to
>
>     buffer->overlays && buffer->overlays->root
>
> Is that correct?  Unless I missed something, the patch description
> doesn't say anything about it.

The admittedly redundant NULL check done by itree_empty_p does not
change semantics or cause harm.  mark_overlays in alloc.c is a recursive
function so it handles NULL itself, hence the NULL check was omitted in
the original code.

Message #32 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Wed, 30 Nov 2022 02:28:09 +0100

Matt Armstrong <matt <at> rfc20.org> writes:

> The admittedly redundant NULL check done by itree_empty_p does not
> change semantics or cause harm.  mark_overlays in alloc.c is a recursive
> function so it handles NULL itself, hence the NULL check was omitted in
> the original code.

Thanks.

I guess as this is a minor cleanup this should go to master at this point.

Message #35 received at 59137 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: matt <at> rfc20.org, monnier <at> iro.umontreal.ca, 59137 <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Wed, 30 Nov 2022 15:25:41 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Wed, 30 Nov 2022 02:28:09 +0100
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>, 59137 <at> debbugs.gnu.org
> 
> Matt Armstrong <matt <at> rfc20.org> writes:
> 
> > The admittedly redundant NULL check done by itree_empty_p does not
> > change semantics or cause harm.  mark_overlays in alloc.c is a recursive
> > function so it handles NULL itself, hence the NULL check was omitted in
> > the original code.
> 
> Thanks.
> 
> I guess as this is a minor cleanup this should go to master at this point.

I agree.  We can always cherry-pick if we find a good reason.

Message #40 received at 59137-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: matt <at> rfc20.org, monnier <at> iro.umontreal.ca, 59137-done <at> debbugs.gnu.org
Subject: Re: bug#59137: [PATCH] To minor changes related to overlays
Date: Wed, 30 Nov 2022 09:34:45 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> I guess as this is a minor cleanup this should go to master at this point.
>
> I agree.  We can always cherry-pick if we find a good reason.

Thanks, pushed to master (commit 656a54b823).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.