[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#17049: [PATCH v2] Make reverse! forego the cost of SCM_VALIDATE_LIST
From: |
Mark H Weaver |
Subject: |
bug#17049: [PATCH v2] Make reverse! forego the cost of SCM_VALIDATE_LIST |
Date: |
Tue, 01 Apr 2014 10:09:32 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
David Kastrup <address@hidden> 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