GNU bug report logs - #17049
[PATCH] Make reverse! forego the cost of SCM_VALIDATE_LIST

Previous Next

Full log

View this message in rfc822 format

From: Mark H Weaver <mhw <at> netris.org>
To: David Kastrup <dak <at> gnu.org>
Cc: 17049 <at> debbugs.gnu.org
Subject: bug#17049: [PATCH v2] Make reverse! forego the cost of SCM_VALIDATE_LIST
Date: Tue, 01 Apr 2014 10:09:32 -0400

David Kastrup <dak <at> gnu.org> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.