unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#59137: [PATCH] To minor changes related to overlays
@ 2022-11-08 23:14 Matt Armstrong
  2022-11-10 10:38 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Armstrong @ 2022-11-08 23:14 UTC (permalink / raw)
  To: 59137

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

Tags: patch

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch --]
[-- Type: text/x-diff, Size: 2809 bytes --]

From 023dddaf723aacd6579331f76f61a2741a4e52d5 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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);
 }
 \f
 /* 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Simplify-ITREE_FOREACH.patch --]
[-- Type: text/x-diff, Size: 1798 bytes --]

From 67caf59c7399659a7de273c175134eac88e777ac Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-08 23:14 bug#59137: [PATCH] To minor changes related to overlays Matt Armstrong
@ 2022-11-10 10:38 ` Eli Zaretskii
  2022-11-15 17:53   ` Matt Armstrong
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2022-11-10 10:38 UTC (permalink / raw)
  To: Matt Armstrong, Stefan Monnier; +Cc: 59137

> From: Matt Armstrong <matt@rfc20.org>
> Date: Tue, 08 Nov 2022 15:14:08 -0800
> 
> Tags: patch
> 
> X-Debbugs-CC: Stefan Monnier <monnier@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.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-10 10:38 ` Eli Zaretskii
@ 2022-11-15 17:53   ` Matt Armstrong
  2022-11-25  1:16     ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Armstrong @ 2022-11-15 17:53 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Monnier; +Cc: 59137

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

Eli Zaretskii <eliz@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch --]
[-- Type: text/x-diff, Size: 2801 bytes --]

From 9cdfe65ee28c68398b9305d15f178358f02f4498 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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);
 }
 \f
 /* 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Simplify-ITREE_FOREACH.patch --]
[-- Type: text/x-diff, Size: 1795 bytes --]

From c2fc83ff7e7d49bdb013881453e504f21f038104 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-15 17:53   ` Matt Armstrong
@ 2022-11-25  1:16     ` Stefan Kangas
  2022-11-25 14:48       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2022-11-25  1:16 UTC (permalink / raw)
  To: Matt Armstrong; +Cc: Eli Zaretskii, Stefan Monnier, 59137

Matt Armstrong <matt@rfc20.org> writes:

> Eli Zaretskii <eliz@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?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-25  1:16     ` Stefan Kangas
@ 2022-11-25 14:48       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-11-26 14:05         ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-11-25 14:48 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Matt Armstrong, Eli Zaretskii, 59137

> 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






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-25 14:48       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-11-26 14:05         ` Stefan Kangas
  2022-11-26 19:37           ` Matt Armstrong
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2022-11-26 14:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Matt Armstrong, Eli Zaretskii, 59137

Stefan Monnier <monnier@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.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-26 14:05         ` Stefan Kangas
@ 2022-11-26 19:37           ` Matt Armstrong
  2022-11-26 20:07             ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Armstrong @ 2022-11-26 19:37 UTC (permalink / raw)
  To: Stefan Kangas, Stefan Monnier; +Cc: Eli Zaretskii, 59137

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

Stefan Kangas <stefankangas@gmail.com> writes:

> Stefan Monnier <monnier@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-itree_empty_p-for-clarity-and-reduced-coupling.patch --]
[-- Type: text/x-diff, Size: 2835 bytes --]

From 3e2c4cd143d51c66198dd606e18015eeae42f3ec Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@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);
 }
 \f
 /* 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-26 19:37           ` Matt Armstrong
@ 2022-11-26 20:07             ` Stefan Kangas
  2022-11-29 23:16               ` Matt Armstrong
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2022-11-26 20:07 UTC (permalink / raw)
  To: Matt Armstrong, Stefan Monnier; +Cc: Eli Zaretskii, 59137

Matt Armstrong <matt@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@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.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-26 20:07             ` Stefan Kangas
@ 2022-11-29 23:16               ` Matt Armstrong
  2022-11-30  1:28                 ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Armstrong @ 2022-11-29 23:16 UTC (permalink / raw)
  To: Stefan Kangas, Stefan Monnier; +Cc: Eli Zaretskii, 59137

Stefan Kangas <stefankangas@gmail.com> writes:

> Matt Armstrong <matt@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@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.






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-29 23:16               ` Matt Armstrong
@ 2022-11-30  1:28                 ` Stefan Kangas
  2022-11-30 13:25                   ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Kangas @ 2022-11-30  1:28 UTC (permalink / raw)
  To: Matt Armstrong; +Cc: Eli Zaretskii, Stefan Monnier, 59137

Matt Armstrong <matt@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.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-30  1:28                 ` Stefan Kangas
@ 2022-11-30 13:25                   ` Eli Zaretskii
  2022-11-30 17:34                     ` Stefan Kangas
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2022-11-30 13:25 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: matt, monnier, 59137

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Wed, 30 Nov 2022 02:28:09 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>, 59137@debbugs.gnu.org
> 
> Matt Armstrong <matt@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.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#59137: [PATCH] To minor changes related to overlays
  2022-11-30 13:25                   ` Eli Zaretskii
@ 2022-11-30 17:34                     ` Stefan Kangas
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Kangas @ 2022-11-30 17:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: matt, monnier, 59137-done

Eli Zaretskii <eliz@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).





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-11-30 17:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-08 23:14 bug#59137: [PATCH] To minor changes related to overlays Matt Armstrong
2022-11-10 10:38 ` Eli Zaretskii
2022-11-15 17:53   ` Matt Armstrong
2022-11-25  1:16     ` Stefan Kangas
2022-11-25 14:48       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-26 14:05         ` Stefan Kangas
2022-11-26 19:37           ` Matt Armstrong
2022-11-26 20:07             ` Stefan Kangas
2022-11-29 23:16               ` Matt Armstrong
2022-11-30  1:28                 ` Stefan Kangas
2022-11-30 13:25                   ` Eli Zaretskii
2022-11-30 17:34                     ` Stefan Kangas

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