[lucy-dev] Error reporting in allot_params

[Marvin Humphrey <ma...@rectangular.com>]

Greets,

The main function we use for converting named arguments in Perl-space to
variables in C-space is XSBind_allot_params(), located in
trunk/perl/xs/XSBind.c.  For the Python people out there, allot_params()
vaguely resembles the PyArg_Parse* functions documented at
<http://docs.python.org/py3k/c-api/arg.html#api-functions> and
<http://docs.python.org/py3k/extending/extending.html>.  (There is no analogue
in the Perl C API.)

There are two improvements I have in mind for allot_params().  The first has
to do with error reporting.

If allot_params() discovers one of the following problems, it throws an
exception:

    * Unrecognized parameter name.
    * Odd number of arguments (i.e. not key-value pairs).
    * Missing parameter name hash.

However, the resulting stack trace does not include the name of the method
where the error occurred.  It mentions allot_params(), but it does not mention
the XS binding caller:

    Invalid parameter: 'bogus'
        cfish_XSBind_allot_params at xs/XSBind.c line 408
        at foo.t line 5

If we change allot_params() to return false and store an exception object in
Err_error on failure, then the calling code can RETHROW the exception...

        bool_t args_ok = XSBind_allot_params( &(ST(0)), 1, 
            items, "Lucy::Search::IndexSearcher::new_PARAMS", 
            &index_sv, "index", 5,
            NULL);
        if (!args_ok) {
            RETHROW(INCREF(Err_get_error()));
        }   

... so that we see its location in the stack trace instead.

    Invalid parameter: 'bogus'
        cfish_XSBind_allot_params at xs/XSBind.c line 408
        XS_Lucy_Search_IndexSearcher_new at lib/Lucy.xs line 1647
        at foo.t line 5

"XS_Lucy_Search_IndexSearcher_new" isn't quite as good as
"Lucy::Search::IndexSearcher::new", but it's better than nothing.

The second improvement I have in mind is to give allot_params() significantly
more responsibility with regards to extracting values.  I'll explain the
details in a separate email.

Marvin Humphrey

Re: [lucy-dev] Error reporting in allot_params

[Peter Karman <pe...@peknet.com>]

Marvin Humphrey wrote on 1/6/11 9:31 PM:

> If we change allot_params() to return false and store an exception object in
> Err_error on failure, then the calling code can RETHROW the exception...
> 
>         bool_t args_ok = XSBind_allot_params( &(ST(0)), 1, 
>             items, "Lucy::Search::IndexSearcher::new_PARAMS", 
>             &index_sv, "index", 5,
>             NULL);
>         if (!args_ok) {
>             RETHROW(INCREF(Err_get_error()));
>         }   
> 
> ... so that we see its location in the stack trace instead.
> 
>     Invalid parameter: 'bogus'
>         cfish_XSBind_allot_params at xs/XSBind.c line 408
>         XS_Lucy_Search_IndexSearcher_new at lib/Lucy.xs line 1647
>         at foo.t line 5
> 
> "XS_Lucy_Search_IndexSearcher_new" isn't quite as good as
> "Lucy::Search::IndexSearcher::new", but it's better than nothing.
> 

+1


-- 
Peter Karman  .  http://peknet.com/  .  peter@peknet.com