Package: emacs;
Reported by: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Fri, 17 Aug 2012 00:14:01 UTC
Severity: normal
Tags: patch
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Paul Eggert <eggert <at> cs.ucla.edu> To: 12215 <at> debbugs.gnu.org Cc: Dmitry Antipov <dmantipov <at> yandex.ru> Subject: bug#12215: CSET is unnecessarily confusing Date: Thu, 16 Aug 2012 17:04:37 -0700
Recent changes to Emacs have introduced code like the following:
CSET (XCHAR_TABLE (char_table), parent, parent);
This is unnecessarily confusing. Those two 'parent' expressions
refer to different things; the first 'parent' is not really a C
expression at all. I recall that Stefan also expressed unease about
CSET's not acting like a function, in this respect.
It's easy to change lisp.h so that the same code can be
written as follows, which is shorter and clearer:
char_table_set_parent (char_table, parent);
The main objection to changing lisp.h, if I recall correctly, is that
it will make lisp.h longer, since lisp.h will need a separate setter
function for each field. But that's not much of a problem since
these functions are really simple. And the advantage of readability
in users of the code makes the .h change worthwhile.
Here's a patch to make this change for CSET. I'd like to install this,
along with similar patches for the other non-function ?SET macros defined
recently.
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2012-08-16 21:58:44 +0000
+++ src/ChangeLog 2012-08-16 23:59:55 +0000
@@ -1,5 +1,14 @@
2012-08-16 Paul Eggert <eggert <at> cs.ucla.edu>
+ * lisp.h (CSET): Remove.
+ (char_table_set_ascii, char_table_set_defalt, char_table_set_parent)
+ (char_table_set_purpose): New functions,
+ replacing CSET. All uses changed. For example, replace
+ "CSET (XCHAR_TABLE (char_table), parent, parent);" with
+ "char_table_set_parent (char_table, parent);".
+ The old version was confusing because it used the same name
+ 'parent' for two different things.
+
Use ASCII tests for character types.
* category.c, dispnew.c, doprnt.c, editfns.c, syntax.c, term.c:
* xfns.c, xterm.c:
=== modified file 'src/casetab.c'
--- src/casetab.c 2012-08-16 03:13:44 +0000
+++ src/casetab.c 2012-08-16 23:59:55 +0000
@@ -260,7 +260,7 @@
down = Fmake_char_table (Qcase_table, Qnil);
Vascii_downcase_table = down;
- CSET (XCHAR_TABLE (down), purpose, Qcase_table);
+ char_table_set_purpose (down, Qcase_table);
for (i = 0; i < 128; i++)
{
=== modified file 'src/category.c'
--- src/category.c 2012-08-16 21:58:44 +0000
+++ src/category.c 2012-08-16 23:59:55 +0000
@@ -238,8 +238,8 @@
table = copy_char_table (table);
if (! NILP (XCHAR_TABLE (table)->defalt))
- CSET (XCHAR_TABLE (table), defalt,
- Fcopy_sequence (XCHAR_TABLE (table)->defalt));
+ char_table_set_defalt (table,
+ Fcopy_sequence (XCHAR_TABLE (table)->defalt));
char_table_set_extras
(table, 0, Fcopy_sequence (XCHAR_TABLE (table)->extras[0]));
map_char_table (copy_category_entry, Qnil, table, table);
@@ -270,7 +270,7 @@
int i;
val = Fmake_char_table (Qcategory_table, Qnil);
- CSET (XCHAR_TABLE (val), defalt, MAKE_CATEGORY_SET);
+ char_table_set_defalt (val, MAKE_CATEGORY_SET);
for (i = 0; i < (1 << CHARTAB_SIZE_BITS_0); i++)
char_table_set_contents (val, i, MAKE_CATEGORY_SET);
Fset_char_table_extra_slot (val, make_number (0),
@@ -466,7 +466,7 @@
Vstandard_category_table = Fmake_char_table (Qcategory_table, Qnil);
/* Set a category set which contains nothing to the default. */
- CSET (XCHAR_TABLE (Vstandard_category_table), defalt, MAKE_CATEGORY_SET);
+ char_table_set_defalt (Vstandard_category_table, MAKE_CATEGORY_SET);
Fset_char_table_extra_slot (Vstandard_category_table, make_number (0),
Fmake_vector (make_number (95), Qnil));
}
=== modified file 'src/chartab.c'
--- src/chartab.c 2012-08-16 07:26:18 +0000
+++ src/chartab.c 2012-08-16 23:59:55 +0000
@@ -115,8 +115,8 @@
size = VECSIZE (struct Lisp_Char_Table) - 1 + n_extras;
vector = Fmake_vector (make_number (size), init);
XSETPVECTYPE (XVECTOR (vector), PVEC_CHAR_TABLE);
- CSET (XCHAR_TABLE (vector), parent, Qnil);
- CSET (XCHAR_TABLE (vector), purpose, purpose);
+ char_table_set_parent (vector, Qnil);
+ char_table_set_purpose (vector, purpose);
XSETCHAR_TABLE (vector, XCHAR_TABLE (vector));
return vector;
}
@@ -185,16 +185,16 @@
copy = Fmake_vector (make_number (size), Qnil);
XSETPVECTYPE (XVECTOR (copy), PVEC_CHAR_TABLE);
- CSET (XCHAR_TABLE (copy), defalt, XCHAR_TABLE (table)->defalt);
- CSET (XCHAR_TABLE (copy), parent, XCHAR_TABLE (table)->parent);
- CSET (XCHAR_TABLE (copy), purpose, XCHAR_TABLE (table)->purpose);
+ char_table_set_defalt (copy, XCHAR_TABLE (table)->defalt);
+ char_table_set_parent (copy, XCHAR_TABLE (table)->parent);
+ char_table_set_purpose (copy, XCHAR_TABLE (table)->purpose);
for (i = 0; i < chartab_size[0]; i++)
char_table_set_contents
- (copy, i,
+ (copy, i,
(SUB_CHAR_TABLE_P (XCHAR_TABLE (table)->contents[i])
? copy_sub_char_table (XCHAR_TABLE (table)->contents[i])
: XCHAR_TABLE (table)->contents[i]));
- CSET (XCHAR_TABLE (copy), ascii, char_table_ascii (copy));
+ char_table_set_ascii (copy, char_table_ascii (copy));
size -= VECSIZE (struct Lisp_Char_Table) - 1;
for (i = 0; i < size; i++)
char_table_set_extras (copy, i, XCHAR_TABLE (table)->extras[i]);
@@ -436,7 +436,7 @@
}
sub_char_table_set (sub, c, val, UNIPROP_TABLE_P (table));
if (ASCII_CHAR_P (c))
- CSET (tbl, ascii, char_table_ascii (table));
+ char_table_set_ascii (table, char_table_ascii (table));
}
return val;
}
@@ -512,7 +512,7 @@
}
}
if (ASCII_CHAR_P (from))
- CSET (tbl, ascii, char_table_ascii (table));
+ char_table_set_ascii (table, char_table_ascii (table));
}
return val;
}
@@ -562,7 +562,7 @@
error ("Attempt to make a chartable be its own parent");
}
- CSET (XCHAR_TABLE (char_table), parent, parent);
+ char_table_set_parent (char_table, parent);
return parent;
}
@@ -640,12 +640,12 @@
{
int i;
- CSET (XCHAR_TABLE (char_table), ascii, value);
+ char_table_set_ascii (char_table, value);
for (i = 0; i < chartab_size[0]; i++)
char_table_set_contents (char_table, i, value);
}
else if (EQ (range, Qnil))
- CSET (XCHAR_TABLE (char_table), defalt, value);
+ char_table_set_defalt (char_table, value);
else if (CHARACTERP (range))
char_table_set (char_table, XINT (range), value);
else if (CONSP (range))
@@ -736,7 +736,7 @@
(char_table, i, optimize_sub_char_table (elt, test));
}
/* Reset the `ascii' cache, in case it got optimized away. */
- CSET (XCHAR_TABLE (char_table), ascii, char_table_ascii (char_table));
+ char_table_set_ascii (char_table, char_table_ascii (char_table));
return Qnil;
}
@@ -828,9 +828,9 @@
/* This is to get a value of FROM in PARENT
without checking the parent of PARENT. */
- CSET (XCHAR_TABLE (parent), parent, Qnil);
+ char_table_set_parent (parent, Qnil);
val = CHAR_TABLE_REF (parent, from);
- CSET (XCHAR_TABLE (parent), parent, temp);
+ char_table_set_parent (parent, temp);
XSETCDR (range, make_number (c - 1));
val = map_sub_char_table (c_function, function,
parent, arg, val, range,
@@ -910,9 +910,9 @@
temp = XCHAR_TABLE (parent)->parent;
/* This is to get a value of FROM in PARENT without checking the
parent of PARENT. */
- CSET (XCHAR_TABLE (parent), parent, Qnil);
+ char_table_set_parent (parent, Qnil);
val = CHAR_TABLE_REF (parent, from);
- CSET (XCHAR_TABLE (parent), parent, temp);
+ char_table_set_parent (parent, temp);
val = map_sub_char_table (c_function, function, parent, arg, val, range,
parent);
table = parent;
@@ -1350,7 +1350,7 @@
: ! NILP (val))
return Qnil;
/* Prepare ASCII values in advance for CHAR_TABLE_REF. */
- CSET (XCHAR_TABLE (table), ascii, char_table_ascii (table));
+ char_table_set_ascii (table, char_table_ascii (table));
return table;
}
=== modified file 'src/fns.c'
--- src/fns.c 2012-08-16 03:13:44 +0000
+++ src/fns.c 2012-08-16 23:59:55 +0000
@@ -2151,7 +2151,7 @@
for (i = 0; i < (1 << CHARTAB_SIZE_BITS_0); i++)
char_table_set_contents (array, i, item);
- CSET (XCHAR_TABLE (array), defalt, item);
+ char_table_set_defalt (array, item);
}
else if (STRINGP (array))
{
=== modified file 'src/fontset.c'
--- src/fontset.c 2012-08-16 03:13:44 +0000
+++ src/fontset.c 2012-08-16 23:59:55 +0000
@@ -1979,7 +1979,7 @@
if (c <= MAX_5_BYTE_CHAR)
char_table_set_range (tables[k], c, to, alist);
else
- CSET (XCHAR_TABLE (tables[k]), defalt, alist);
+ char_table_set_defalt (tables[k], alist);
/* At last, change each elements to font names. */
for (; CONSP (alist); alist = XCDR (alist))
=== modified file 'src/lisp.h'
--- src/lisp.h 2012-08-16 07:58:24 +0000
+++ src/lisp.h 2012-08-16 23:59:55 +0000
@@ -936,12 +936,6 @@
extern const int chartab_size[4];
-/* Most code should use this macro to set non-array Lisp fields in struct
- Lisp_Char_Table. For CONTENTS and EXTRAS, use char_table_set_contents
- and char_table_set_extras, respectively. */
-
-#define CSET(c, field, value) ((c)->field = (value))
-
struct Lisp_Char_Table
{
/* HEADER.SIZE is the vector's size field, which also holds the
@@ -2439,6 +2433,30 @@
XSTRING (s)->intervals = i;
}
+/* Set a Lisp slot in TABLE to VAL. Most code should use this instead
+ of setting slots directly. */
+
+LISP_INLINE void
+char_table_set_ascii (Lisp_Object table, Lisp_Object val)
+{
+ XCHAR_TABLE (table)->ascii = val;
+}
+LISP_INLINE void
+char_table_set_defalt (Lisp_Object table, Lisp_Object val)
+{
+ XCHAR_TABLE (table)->defalt = val;
+}
+LISP_INLINE void
+char_table_set_parent (Lisp_Object table, Lisp_Object val)
+{
+ XCHAR_TABLE (table)->parent = val;
+}
+LISP_INLINE void
+char_table_set_purpose (Lisp_Object table, Lisp_Object val)
+{
+ XCHAR_TABLE (table)->purpose = val;
+}
+
/* Set different slots in (sub)character tables. */
LISP_INLINE void
=== modified file 'src/syntax.c'
--- src/syntax.c 2012-08-16 21:58:44 +0000
+++ src/syntax.c 2012-08-16 23:59:55 +0000
@@ -818,7 +818,7 @@
/* Only the standard syntax table should have a default element.
Other syntax tables should inherit from parents instead. */
- CSET (XCHAR_TABLE (copy), defalt, Qnil);
+ char_table_set_defalt (copy, Qnil);
/* Copied syntax tables should all have parents.
If we copied one with no parent, such as the standard syntax table,
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.