From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#12215: CSET is unnecessarily confusing Date: Thu, 16 Aug 2012 17:04:37 -0700 Organization: UCLA Computer Science Department Message-ID: <502D8A95.7060609@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1345161921 16460 80.91.229.3 (17 Aug 2012 00:05:21 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 17 Aug 2012 00:05:21 +0000 (UTC) Cc: Dmitry Antipov To: 12215@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Aug 17 02:05:20 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1T2A3w-0000tW-EY for geb-bug-gnu-emacs@m.gmane.org; Fri, 17 Aug 2012 02:05:16 +0200 Original-Received: from localhost ([::1]:44072 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3u-0005cr-QU for geb-bug-gnu-emacs@m.gmane.org; Thu, 16 Aug 2012 20:05:14 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:40637) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3s-0005cm-8O for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:05:13 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T2A3q-0005MY-NJ for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:05:12 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:53029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3q-0005MQ-Ik for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:05:10 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1T2ACP-0005zQ-Uq for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:14:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 17 Aug 2012 00:14:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 12215 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.134516242022984 (code B ref -1); Fri, 17 Aug 2012 00:14:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 17 Aug 2012 00:13:40 +0000 Original-Received: from localhost ([127.0.0.1]:34342 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1T2AC2-0005ye-V2 for submit@debbugs.gnu.org; Thu, 16 Aug 2012 20:13:39 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:50759) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1T2ABz-0005yW-Rw for submit@debbugs.gnu.org; Thu, 16 Aug 2012 20:13:38 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T2A3O-00058K-9Q for submit@debbugs.gnu.org; Thu, 16 Aug 2012 20:04:43 -0400 Original-Received: from lists.gnu.org ([208.118.235.17]:53646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3O-00058G-6F for submit@debbugs.gnu.org; Thu, 16 Aug 2012 20:04:42 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:59564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3M-0005Y8-Gz for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:04:42 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T2A3K-000582-Ph for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:04:40 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:56928) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T2A3K-00057w-Er for bug-gnu-emacs@gnu.org; Thu, 16 Aug 2012 20:04:38 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id A4455A60006; Thu, 16 Aug 2012 17:04:37 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OyX49VCB0KnQ; Thu, 16 Aug 2012 17:04:36 -0700 (PDT) Original-Received: from [192.168.1.3] (pool-108-23-119-2.lsanca.fios.verizon.net [108.23.119.2]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 71739A60004; Thu, 16 Aug 2012 17:04:36 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20120714 Thunderbird/14.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:63235 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 + * 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,