Re: [PATCH] Implement 'scm_c_bind_kwargs' to handle keyword arguments from C

[Mark H Weaver <mhw@netris.org>]

Hi Andy,

Andy Wingo <wingo@pobox.com> 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