Subject: bug#45550: [PATCH] Factor out new function for readability in chartab.c

From: Stefan Kangas <[email-masked]>
Date: Wed, 30 Dec 2020 02:56:53 -0600

Severity: wishlist

While poking around, I found an opportunity for refactoring in
charctab.c.  I find it makes these functions significantly easier to
read and understand.

Please see the attached patch.
From 2365011892e4587cfdd3c3f0d0501c4afef2c441 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <[email-masked]>
Date: Wed, 30 Dec 2020 10:14:20 +0100
Subject: [PATCH] Factor out new function for readability

* src/chartab.c (sub_char_table_ref_and_range): New function...
(sub_char_table_ref_and_range, char_table_ref_and_range):
...factored out from here.
---
 src/chartab.c | 104 +++++++++++++++++++++-----------------------------
 1 file changed, 44 insertions(+), 60 deletions(-)

diff --git a/src/chartab.c b/src/chartab.c
index 331e8595eb..933853ed18 100644
--- a/src/chartab.c
+++ b/src/chartab.c
@@ -62,6 +62,9 @@ #define CHARTAB_IDX(c, depth, min_char)		\
 
 static Lisp_Object uniprop_table_uncompress (Lisp_Object, int);
 static uniprop_decoder_t uniprop_get_decoder (Lisp_Object);
+static Lisp_Object
+sub_char_table_ref_and_range (Lisp_Object, int, int *, int *,
+			      Lisp_Object, bool);
 
 /* 1 iff TABLE is a uniprop table.  */
 #define UNIPROP_TABLE_P(TABLE)					\
@@ -247,6 +250,23 @@ char_table_ref (Lisp_Object table, int c)
   return val;
 }
 
+static Lisp_Object
+char_table_ref_simple (Lisp_Object table, int idx, int c, int *from, int *to,
+		       Lisp_Object defalt, bool is_uniprop, bool is_subtable)
+{
+  Lisp_Object val = is_subtable ?
+    XSUB_CHAR_TABLE (table)->contents[idx]:
+    XCHAR_TABLE (table)->contents[idx];
+  if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (val))
+    val = uniprop_table_uncompress (table, idx);
+  if (SUB_CHAR_TABLE_P (val))
+    val = sub_char_table_ref_and_range (val, c, from, to,
+					defalt, is_uniprop);
+  else if (NILP (val))
+    val = defalt;
+  return val;
+}
+
 static Lisp_Object
 sub_char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to,
 			      Lisp_Object defalt, bool is_uniprop)
@@ -254,31 +274,18 @@ sub_char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to,
   struct Lisp_Sub_Char_Table *tbl = XSUB_CHAR_TABLE (table);
   int depth = tbl->depth, min_char = tbl->min_char;
   int chartab_idx = CHARTAB_IDX (c, depth, min_char), idx;
-  Lisp_Object val;
-
-  val = tbl->contents[chartab_idx];
-  if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (val))
-    val = uniprop_table_uncompress (table, chartab_idx);
-  if (SUB_CHAR_TABLE_P (val))
-    val = sub_char_table_ref_and_range (val, c, from, to, defalt, is_uniprop);
-  else if (NILP (val))
-    val = defalt;
+  Lisp_Object val
+    = char_table_ref_simple (table, chartab_idx, c, from, to,
+			     defalt, is_uniprop, true);
 
   idx = chartab_idx;
   while (idx > 0 && *from < min_char + idx * chartab_chars[depth])
     {
-      Lisp_Object this_val;
-
       c = min_char + idx * chartab_chars[depth] - 1;
       idx--;
-      this_val = tbl->contents[idx];
-      if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (this_val))
-	this_val = uniprop_table_uncompress (table, idx);
-      if (SUB_CHAR_TABLE_P (this_val))
-	this_val = sub_char_table_ref_and_range (this_val, c, from, to, defalt,
-						 is_uniprop);
-      else if (NILP (this_val))
-	this_val = defalt;
+      Lisp_Object this_val
+	= char_table_ref_simple (table, idx, c, from, to,
+				 defalt, is_uniprop, true);
 
       if (! EQ (this_val, val))
 	{
@@ -290,17 +297,11 @@ sub_char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to,
 	  < chartab_chars[depth - 1])
 	 && (c += min_char) <= *to)
     {
-      Lisp_Object this_val;
-
       chartab_idx++;
-      this_val = tbl->contents[chartab_idx];
-      if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (this_val))
-	this_val = uniprop_table_uncompress (table, chartab_idx);
-      if (SUB_CHAR_TABLE_P (this_val))
-	this_val = sub_char_table_ref_and_range (this_val, c, from, to, defalt,
-						 is_uniprop);
-      else if (NILP (this_val))
-	this_val = defalt;
+      Lisp_Object this_val
+	= char_table_ref_simple (table, chartab_idx, c, from, to,
+				 defalt, is_uniprop, true);
+
       if (! EQ (this_val, val))
 	{
 	  *to = c - 1;
@@ -321,37 +322,26 @@ sub_char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to,
 char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to)
 {
   struct Lisp_Char_Table *tbl = XCHAR_TABLE (table);
-  int chartab_idx = CHARTAB_IDX (c, 0, 0), idx;
-  Lisp_Object val;
+  int chartab_idx = CHARTAB_IDX (c, 0, 0);
   bool is_uniprop = UNIPROP_TABLE_P (table);
 
-  val = tbl->contents[chartab_idx];
   if (*from < 0)
     *from = 0;
   if (*to < 0)
     *to = MAX_CHAR;
-  if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (val))
-    val = uniprop_table_uncompress (table, chartab_idx);
-  if (SUB_CHAR_TABLE_P (val))
-    val = sub_char_table_ref_and_range (val, c, from, to, tbl->defalt,
-					is_uniprop);
-  else if (NILP (val))
-    val = tbl->defalt;
-  idx = chartab_idx;
+
+  Lisp_Object val
+    = char_table_ref_simple (table, chartab_idx, c, from, to,
+			     tbl->defalt, is_uniprop, false);
+
+  int idx = chartab_idx;
   while (*from < idx * chartab_chars[0])
     {
-      Lisp_Object this_val;
-
       c = idx * chartab_chars[0] - 1;
       idx--;
-      this_val = tbl->contents[idx];
-      if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (this_val))
-	this_val = uniprop_table_uncompress (table, idx);
-      if (SUB_CHAR_TABLE_P (this_val))
-	this_val = sub_char_table_ref_and_range (this_val, c, from, to,
-						 tbl->defalt, is_uniprop);
-      else if (NILP (this_val))
-	this_val = tbl->defalt;
+      Lisp_Object this_val
+	= char_table_ref_simple (table, idx, c, from, to,
+				 tbl->defalt, is_uniprop, false);
 
       if (! EQ (this_val, val))
 	{
@@ -361,18 +351,12 @@ char_table_ref_and_range (Lisp_Object table, int c, int *from, int *to)
     }
   while (*to >= (chartab_idx + 1) * chartab_chars[0])
     {
-      Lisp_Object this_val;
-
       chartab_idx++;
       c = chartab_idx * chartab_chars[0];
-      this_val = tbl->contents[chartab_idx];
-      if (is_uniprop && UNIPROP_COMPRESSED_FORM_P (this_val))
-	this_val = uniprop_table_uncompress (table, chartab_idx);
-      if (SUB_CHAR_TABLE_P (this_val))
-	this_val = sub_char_table_ref_and_range (this_val, c, from, to,
-						 tbl->defalt, is_uniprop);
-      else if (NILP (this_val))
-	this_val = tbl->defalt;
+      Lisp_Object this_val
+	= char_table_ref_simple (table, chartab_idx, c, from, to,
+				 tbl->defalt, is_uniprop, false);
+
       if (! EQ (this_val, val))
 	{
 	  *to = c - 1;
-- 
2.29.2