From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Implement 'scm_c_bind_kwargs' to handle keyword arguments from C Date: Sat, 06 Apr 2013 21:23:21 -0400 Message-ID: <87k3oft9py.fsf@tines.lan> References: <87vc7ztq01.fsf@tines.lan> <87fvz35r2v.fsf@pobox.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1365297831 3151 80.91.229.3 (7 Apr 2013 01:23:51 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 7 Apr 2013 01:23:51 +0000 (UTC) Cc: guile-devel@gnu.org To: Andy Wingo Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Sun Apr 07 03:23:55 2013 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 1UOeKj-0004M1-Ky for guile-devel@m.gmane.org; Sun, 07 Apr 2013 03:23:49 +0200 Original-Received: from localhost ([::1]:58801 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UOeKj-0000lT-8J for guile-devel@m.gmane.org; Sat, 06 Apr 2013 21:23:49 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:57000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UOeKb-0000lL-68 for guile-devel@gnu.org; Sat, 06 Apr 2013 21:23:45 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UOeKX-0005n2-4o for guile-devel@gnu.org; Sat, 06 Apr 2013 21:23:41 -0400 Original-Received: from world.peace.net ([96.39.62.75]:34945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UOeKX-0005mw-0G for guile-devel@gnu.org; Sat, 06 Apr 2013 21:23:37 -0400 Original-Received: from 209-6-91-212.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.91.212] helo=tines.lan) by world.peace.net with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1UOeKQ-0006OZ-0t; Sat, 06 Apr 2013 21:23:30 -0400 In-Reply-To: <87fvz35r2v.fsf@pobox.com> (Andy Wingo's message of "Sat, 06 Apr 2013 22:42:16 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 96.39.62.75 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:16178 Archived-At: Hi Andy, Andy Wingo writes: > I'm OK with this in principle, but we shouldagree on names before this > goes in. Indeed, I often have trouble coming up with good names. We talked about it on IRC, and agreed on 'scm_c_bind_keyword_arguments', 'SCM_ALLOW_OTHER_KEYS' and 'SCM_ALLOW_NON_KEYWORD_ARGUMENTS', which are certainly better than what I had. >> + va_start (va, allow_other_keys); > > "flags", no? Yes, I noticed that shortly after posting. Oops! :) >> + for (;;) >> + { >> + kw = va_arg (va, SCM); >> + if (SCM_UNBNDP (kw)) >> + { >> + /* KW_OR_ARG is not in the list of expected keywords. */ >> + if (!allow_other_keys) >> + scm_error (scm_keyword_argument_error, >> + subr, "Unrecognized keyword", >> + SCM_EOL, SCM_BOOL_F); >> + break; >> + } > > Don't we need to advance "tail" in the "allow_other_keys" case, to skip > over the argument value? That is what the bind-kwargs VM op does. 'tail' is not used again. 'rest' is advanced further down in the code. >> + /* The next argument is not a keyword, or is a singleton >> + keyword at the end of REST. */ >> + if (!allow_rest) >> + scm_error (scm_keyword_argument_error, >> + subr, "Invalid keyword", >> + SCM_EOL, SCM_BOOL_F); >> + >> + /* Advance REST. */ >> + rest = tail; > > I think the semantics of rest arguments with keywords is that the rest > argument *includes* the keywords. Agreed. 'rest' is not returned, but just used internally to iterate over the list. As you suggested, I switched to using an enum for the flags. You gave your blessing on IRC for me to push this after these updates, so I did so. Thanks! Mark