all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Change a while loop into do while
@ 2016-12-27  4:55 Chris Gregory
  2016-12-31 11:20 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Gregory @ 2016-12-27  4:55 UTC (permalink / raw)
  To: emacs-devel

This patch changes string_from_display_spec (a static function in
xdisp.c) to use a do while loop instead of a while loop.  The
precondition is checked by the if statement and is redundant in the
first iteration.  This redundancy is now removed.

diff --git a/src/xdisp.c b/src/xdisp.c
index 5de5eca..ac7a1f3 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1252,12 +1252,13 @@ string_from_display_spec (Lisp_Object spec)
 {
   if (CONSP (spec))
     {
-      while (CONSP (spec))
+      do
 	{
 	  if (STRINGP (XCAR (spec)))
 	    return XCAR (spec);
 	  spec = XCDR (spec);
 	}
+      while (CONSP (spec));
     }
   else if (VECTORP (spec))
     {



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

* Re: Change a while loop into do while
  2016-12-27  4:55 Change a while loop into do while Chris Gregory
@ 2016-12-31 11:20 ` Eli Zaretskii
  2016-12-31 17:20   ` Paul Eggert
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 11:20 UTC (permalink / raw)
  To: Chris Gregory; +Cc: emacs-devel

> From: Chris Gregory <czipperz@gmail.com>
> Date: Mon, 26 Dec 2016 22:55:54 -0600
> 
> This patch changes string_from_display_spec (a static function in
> xdisp.c) to use a do while loop instead of a while loop.  The
> precondition is checked by the if statement and is redundant in the
> first iteration.  This redundancy is now removed.

Thanks, pushed to the master branch.

In the future, please provide ChangeLog-style commit log messages with
your changes (I wrote one this time).



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

* Re: Change a while loop into do while
  2016-12-31 11:20 ` Eli Zaretskii
@ 2016-12-31 17:20   ` Paul Eggert
  2016-12-31 18:00     ` Eli Zaretskii
  2016-12-31 18:12     ` Eli Zaretskii
  0 siblings, 2 replies; 11+ messages in thread
From: Paul Eggert @ 2016-12-31 17:20 UTC (permalink / raw)
  To: Eli Zaretskii, Chris Gregory; +Cc: emacs-devel

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

> From: Chris Gregory <czipperz@gmail.com>
> Date: Mon, 26 Dec 2016 22:55:54 -0600
>
> This patch changes string_from_display_spec (a static function in
> xdisp.c) to use a do while loop instead of a while loop.  The
> precondition is checked by the if statement and is redundant in the
> first iteration.  This redundancy is now removed.

The indenting for 'do-while' wasn't using the usual GNU style, and while fixing 
that I noticed that the code could be further simplified, so I installed the 
attached further patch.

[-- Attachment #2: 0001-src-xdisp.c-string_from_display_spec-Simplify.patch --]
[-- Type: text/x-diff, Size: 1185 bytes --]

From c48d410258353bc1ed8ec658ab69da2d67e1e8ea Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 31 Dec 2016 09:13:38 -0800
Subject: [PATCH] * src/xdisp.c (string_from_display_spec): Simplify.

---
 src/xdisp.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 45a04ca..aced59e 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1250,26 +1250,18 @@ default_line_pixel_height (struct window *w)
 static Lisp_Object
 string_from_display_spec (Lisp_Object spec)
 {
-  if (CONSP (spec))
+  if (VECTORP (spec))
     {
-      do {
-	if (STRINGP (XCAR (spec)))
-	  return XCAR (spec);
-	spec = XCDR (spec);
-      } while (CONSP (spec));
+      for (ptrdiff_t i = 0; i < ASIZE (spec); i++)
+	if (STRINGP (AREF (spec, i)))
+	  return AREF (spec, i);
     }
-  else if (VECTORP (spec))
+  else
     {
-      ptrdiff_t i;
-
-      for (i = 0; i < ASIZE (spec); i++)
-	{
-	  if (STRINGP (AREF (spec, i)))
-	    return AREF (spec, i);
-	}
-      return Qnil;
+      for (; CONSP (spec); spec = XCDR (spec))
+	if (STRINGP (XCAR (spec)))
+	  return XCAR (spec);
     }
-
   return spec;
 }
 
-- 
2.7.4


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

* Re: Change a while loop into do while
  2016-12-31 17:20   ` Paul Eggert
@ 2016-12-31 18:00     ` Eli Zaretskii
  2016-12-31 18:51       ` Paul Eggert
  2016-12-31 18:12     ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 18:00 UTC (permalink / raw)
  To: Paul Eggert; +Cc: czipperz, emacs-devel

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 31 Dec 2016 09:20:31 -0800
> 
> The indenting for 'do-while' wasn't using the usual GNU style

Yes, it is: you will see a lot of that elsewhere in Emacs.



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

* Re: Change a while loop into do while
  2016-12-31 17:20   ` Paul Eggert
  2016-12-31 18:00     ` Eli Zaretskii
@ 2016-12-31 18:12     ` Eli Zaretskii
  2016-12-31 18:31       ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 18:12 UTC (permalink / raw)
  To: Paul Eggert; +Cc: czipperz, emacs-devel

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 31 Dec 2016 09:20:31 -0800
> Cc: emacs-devel@gnu.org
> 
> while fixing that I noticed that the code could be further
> simplified, so I installed the attached further patch.

These changes seem to change the value returned by the function when
the argument is a cons cell or a vector, so the new code is not
equivalent to the old one.



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

* Re: Change a while loop into do while
  2016-12-31 18:12     ` Eli Zaretskii
@ 2016-12-31 18:31       ` Eli Zaretskii
  2016-12-31 18:47         ` Paul Eggert
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 18:31 UTC (permalink / raw)
  To: eggert; +Cc: czipperz, emacs-devel

> Date: Sat, 31 Dec 2016 20:12:10 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: czipperz@gmail.com, emacs-devel@gnu.org
> 
> These changes seem to change the value returned by the function when
> the argument is a cons cell or a vector

Sorry, only the vector case will now yield a different result.



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

* Re: Change a while loop into do while
  2016-12-31 18:31       ` Eli Zaretskii
@ 2016-12-31 18:47         ` Paul Eggert
  0 siblings, 0 replies; 11+ messages in thread
From: Paul Eggert @ 2016-12-31 18:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: czipperz, emacs-devel

Eli Zaretskii wrote:
> Sorry, only the vector case will now yield a different result.

Sure, but the caller cares only whether the result is a string, so there's no 
difference whether this function returns Qnil or its arg unchanged in that case, 
and returning its arg unchanged is slightly simpler.



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

* Re: Change a while loop into do while
  2016-12-31 18:00     ` Eli Zaretskii
@ 2016-12-31 18:51       ` Paul Eggert
  2016-12-31 19:18         ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Eggert @ 2016-12-31 18:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: czipperz, emacs-devel

Eli Zaretskii wrote:
>> The indenting for 'do-while' wasn't using the usual GNU style
> Yes, it is: you will see a lot of that elsewhere in Emacs.

It's not the usual GNU style.  The GNU coding standards say to format do-while 
statements like this:

   do
     {
       a = foo (a);
     }
   while (a > 0);

There is a common exception in macros where the do-while is simply a statement 
wrapper, but that exception didn't apply here.



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

* Re: Change a while loop into do while
  2016-12-31 18:51       ` Paul Eggert
@ 2016-12-31 19:18         ` Eli Zaretskii
  2016-12-31 20:56           ` Paul Eggert
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 19:18 UTC (permalink / raw)
  To: Paul Eggert; +Cc: czipperz, emacs-devel

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 31 Dec 2016 10:51:29 -0800
> Cc: czipperz@gmail.com, emacs-devel@gnu.org
> 
> Eli Zaretskii wrote:
> >> The indenting for 'do-while' wasn't using the usual GNU style
> > Yes, it is: you will see a lot of that elsewhere in Emacs.
> 
> It's not the usual GNU style.

Maybe so, but it's widespread enough in Emacs sources, including
outside of any macros, so it's a de-facto standard here.



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

* Re: Change a while loop into do while
  2016-12-31 19:18         ` Eli Zaretskii
@ 2016-12-31 20:56           ` Paul Eggert
  2016-12-31 20:59             ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Eggert @ 2016-12-31 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: czipperz, emacs-devel

Eli Zaretskii wrote:
>> It's not the usual GNU style.
> Maybe so, but it's widespread enough in Emacs sources, including
> outside of any macros, so it's a de-facto standard here.

No, the de facto standard even in Emacs sources is the usual GNU style. Outside 
of macro statements I count 142 instances of the usual GNU style, as opposed to 
only 28 instances of the K&R-ish style where "do {" is on the same line.

Surely if we want to change the GNU style, we should change the coding standards 
rather than just ignore them.



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

* Re: Change a while loop into do while
  2016-12-31 20:56           ` Paul Eggert
@ 2016-12-31 20:59             ` Eli Zaretskii
  0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-12-31 20:59 UTC (permalink / raw)
  To: Paul Eggert; +Cc: czipperz, emacs-devel

> Cc: czipperz@gmail.com, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 31 Dec 2016 12:56:35 -0800
> 
> Eli Zaretskii wrote:
> >> It's not the usual GNU style.
> > Maybe so, but it's widespread enough in Emacs sources, including
> > outside of any macros, so it's a de-facto standard here.
> 
> No, the de facto standard even in Emacs sources is the usual GNU style. Outside 
> of macro statements I count 142 instances of the usual GNU style, as opposed to 
> only 28 instances of the K&R-ish style where "do {" is on the same line.

28 instances are more than enough for me.



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

end of thread, other threads:[~2016-12-31 20:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-27  4:55 Change a while loop into do while Chris Gregory
2016-12-31 11:20 ` Eli Zaretskii
2016-12-31 17:20   ` Paul Eggert
2016-12-31 18:00     ` Eli Zaretskii
2016-12-31 18:51       ` Paul Eggert
2016-12-31 19:18         ` Eli Zaretskii
2016-12-31 20:56           ` Paul Eggert
2016-12-31 20:59             ` Eli Zaretskii
2016-12-31 18:12     ` Eli Zaretskii
2016-12-31 18:31       ` Eli Zaretskii
2016-12-31 18:47         ` Paul Eggert

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.