From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Per-port read options, reader directives, SRFI-105 Date: Tue, 23 Oct 2012 23:26:34 +0200 Message-ID: <87pq4898xh.fsf@gnu.org> References: <87a9vmvh9s.fsf@tines.lan> <87mwzdzpqf.fsf@tines.lan> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1351027667 31135 80.91.229.3 (23 Oct 2012 21:27:47 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 23 Oct 2012 21:27:47 +0000 (UTC) To: guile-devel@gnu.org Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Tue Oct 23 23:27:53 2012 Return-path: Envelope-to: guile-devel@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 1TQm0t-0004Lr-Ll for guile-devel@m.gmane.org; Tue, 23 Oct 2012 23:27:51 +0200 Original-Received: from localhost ([::1]:60317 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQm0m-0002cR-2j for guile-devel@m.gmane.org; Tue, 23 Oct 2012 17:27:44 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:39335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQm0L-0002aM-FF for guile-devel@gnu.org; Tue, 23 Oct 2012 17:27:41 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQlzs-0006tY-LR for guile-devel@gnu.org; Tue, 23 Oct 2012 17:27:17 -0400 Original-Received: from plane.gmane.org ([80.91.229.3]:44506) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQlzs-0006t6-BO for guile-devel@gnu.org; Tue, 23 Oct 2012 17:26:48 -0400 Original-Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1TQlzw-0003fh-AZ for guile-devel@gnu.org; Tue, 23 Oct 2012 23:26:52 +0200 Original-Received: from reverse-83.fdn.fr ([80.67.176.83]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 23 Oct 2012 23:26:52 +0200 Original-Received: from ludo by reverse-83.fdn.fr with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 23 Oct 2012 23:26:52 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 121 Original-X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: reverse-83.fdn.fr X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 2 Brumaire an 221 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0xEA52ECF4 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 83C4 F8E5 10A3 3B4C 5BEA D15D 77DD 95E2 EA52 ECF4 X-OS: x86_64-unknown-linux-gnu User-Agent: Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux) Cancel-Lock: sha1:nakE2fjPX7m0eAKRZOOstlSOSK4= X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 80.91.229.3 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:15036 Archived-At: Mark H Weaver skribis: > From 255aaaf0f474d45bd67d6b3b102b2806a8f0db97 Mon Sep 17 00:00:00 2001 > From: Mark H Weaver > Date: Tue, 23 Oct 2012 00:50:42 -0400 > Subject: [PATCH 7/9] Implement per-port read options. > > * libguile/read.c (scm_t_read_opts): Update comment to mention the > per-port read options. > > (sym_read_option_overrides): New symbol. > > (set_per_port_read_option): New internal static function. The patch add this static function, but leaves it unused. And also, there are no tests. So, what about exposing a ‘set-port-read-options!’ procedure, and then using it to write tests? > (init_read_options): Add new 'port' parameter, and consult the > per-port read option overrides when initializing the 'scm_t_read_opts' > struct. > > (scm_read): Pass 'port' parameter to init_read_options. > > * doc/ref/api-evaluation.texi (Scheme Read): Mention the existence of > (currently unused) per-port reader options. OK. > +Note that Guile also includes a preliminary mechanism for overriding > +read options on a per-port basis, but it is currently unused and there > +is no way to access or set these per-port read options. I think you can remove this paragraph because it becomes largely invalid with the next few patches. > +/* > + * Per-port read option overrides. > + * > + * We store per-port read option overrides in the > + * '%read-option-overrides%' key of the port's alist, which is stored in > + * 'scm_i_port_weak_hash'. The value stored in the alist is a single > + * integer that contains a two-bit field for each read option. > + * > + * If a bit field contains OVERRIDE_DEFAULT (3), that indicates that the > + * corresponding read option has not been overridden for this port, so > + * the global read option should be used. Otherwise, the bit field > + * contains the value of the read option. For boolean read options that > + * have been overridden, the other possible values are 0 or 1. If the > + * 'keyword_style' read option is overridden, its possible values are > + * taken from the enum of the 'scm_t_read_opts' struct. > + */ Tricky semantics, but I guess there’s no other way. > +SCM_SYMBOL (sym_read_option_overrides, "%read-option-overrides%"); Maybe ‘read-option-overrides’ is enough since it’s an internal alist anyway. > +/* Offsets of bit fields for each per-port override */ > +#define OVERRIDE_SHIFT_COPY_SOURCE_P 0 > +#define OVERRIDE_SHIFT_RECORD_POSITIONS_P 2 > +#define OVERRIDE_SHIFT_CASE_INSENSITIVE_P 4 > +#define OVERRIDE_SHIFT_KEYWORD_STYLE 6 > +#define OVERRIDE_SHIFT_R6RS_ESCAPES_P 8 > +#define OVERRIDE_SHIFT_SQUARE_BRACKETS_P 10 > +#define OVERRIDE_SHIFT_HUNGRY_EOL_ESCAPES_P 12 > +#define OVERRIDES_SHIFT_END 14 > + > +#define OVERRIDES_ALL_DEFAULTS ((1UL << OVERRIDES_SHIFT_END) - 1) > +#define OVERRIDES_MAX_VALUE OVERRIDES_ALL_DEFAULTS > + > +#define OVERRIDE_MASK 3 > +#define OVERRIDE_DEFAULT 3 What about s/OVERRIDE_SHIFT/READ_OPTION/? And: > +set_per_port_read_option (SCM port, int shift, int value) Also change ‘shift’ to ‘option’, and ‘int value’ to something like ‘enum t_option_state value’, where: enum t_option_state { OPTION_INHERITED, /* global option setting inherited */ OPTION_DISABLED, OPTION_ENABLED }; the goal being to hide as much of the bit-twiddling as possible. What about also adding: static int per_port_read_option (SCM port, int option); static int applicable_read_option (SCM port, int option); (Maybe it comes next?) > +/* Initialize the internal read options structure from the global and > + per-port read options. */ > +static void > +init_read_options (SCM port, scm_t_read_opts *opts) Rather along the lines of “Initialize OPTS based on PORT’s read options and the global read options.” > +#define RESOLVE_BOOLEAN_OPTION(NAME, name) \ > + do { \ > + x = OVERRIDE_MASK & (overrides >> OVERRIDE_SHIFT_ ## NAME); \ > + if (x == OVERRIDE_DEFAULT) \ > + x = !!SCM_ ## NAME; \ > + opts->name = x; \ > + } while (0) Braces misplaced. Thanks, Ludo’.