Subject: Re: [PATCH v2] Make reverse! forego the cost of SCM_VALIDATE_LIST

From: Mark H Weaver <[email-masked]>
Date: Tue, 01 Apr 2014 10:09:32 -0400

David Kastrup <[email-masked]> writes:

> Sigh.  In case you are committing this, there might be one minor change
> worth doing to match the coding style:
>
>> +  scm_wrong_type_arg (FUNC_NAME, 1, lst);
>
> This should likely be
>
>      SCM_WRONG_TYPE_ARG (1, lst);
>
> for consistency with other usage in list.c.  I can reroll if you prefer
> this over fixing it yourself.

Please reroll, since there's also make a change to the commit log I'd
like.  You wrote:

> * libguile/list.c (scm_reverse_x): Do not check first argument to
> reverse! to be a proper list in advance.  This provides the
> performance of a version without validation (tests show a speedup by a
> factor of 1.8) except for the error case.

As the GNU coding standards suggest, we prefer for change log entries to
contain only a brief description of what changes were made, and to leave
rationales and other explanations in the code comments.

Therefore, I think you should remove the second sentence above, and also
add a brief mention about undoing the reversal if the argument turned
out to be improper.

     Thanks!
       Mark