[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Implement 'scm_c_bind_kwargs' to handle keyword arguments fr
From: |
Andy Wingo |
Subject: |
Re: [PATCH] Implement 'scm_c_bind_kwargs' to handle keyword arguments from C |
Date: |
Sat, 06 Apr 2013 22:42:16 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Hi Mark,
I'm OK with this in principle, but we shouldagree on names before this
goes in.
On Sat 06 Apr 2013 21:31, Mark H Weaver <address@hidden> writes:
> * libguile/keywords.c (scm_keyword_argument_error): New variable.
> (scm_c_bind_kwargs): New API function.
I think I prefer scm_c_bind_keyword_arguments, if that's OK with you.
> +scm_c_bind_kwargs (const char *subr, SCM rest, int flags, ...)
> +{
> + int allow_other_keys = flags & SCM_KWARGS_ALLOW_OTHER_KEYS;
> + int allow_rest = flags & SCM_KWARGS_ALLOW_REST;
"Allow rest" sorta sounds like the #:rest bit from lambda*, but it
doesn't actually help us to bind a rest argument, because we already
have it. Better to call it "ALLOW_NON_KEYWORDS" or "PERMISSIVE" or
"SLOPPY" or something.
> + if (SCM_UNLIKELY (!allow_rest && scm_ilength (rest) % 2 != 0))
> + scm_error (scm_keyword_argument_error,
> + subr, "Odd length of keyword argument list",
> + SCM_EOL, SCM_BOOL_F);
> +
> + while (scm_is_pair (rest))
> + {
> + SCM kw_or_arg = SCM_CAR (rest);
> + SCM tail = SCM_CDR (rest);
> +
> + if (scm_is_keyword (kw_or_arg) && scm_is_pair (tail))
> + {
> + SCM kw;
> + SCM *arg_p;
> +
> + va_start (va, allow_other_keys);
"flags", no?
> + 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.
> + /* 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.
> +#define SCM_KWARGS_ALLOW_OTHER_KEYS 1
> +#define SCM_KWARGS_ALLOW_REST 2
typedef enum
{
SCM_KEYWORD_ARGUMENTS_NO_FLAGS = 0,
SCM_KEYWORD_ARGUMENTS_ALLOW_OTHER_KEYS = (1U << 0),
SCM_KEYWORD_ARGUMENTS_IGNORE_NON_KEYWORDS = (1U << 1)
} scm_t_keyword_arguments_flag;
Cheers,
Andy
--
http://wingolog.org/