Re: [PATCH]: Add new bytecode op `switch' for implementing branch tables.

[Vibhav Pant]

Is there anything else to be done?

On Sat, Feb 11, 2017 at 8:37 PM, Vibhav Pant <address@hidden> wrote:
> On Sat, Feb 11, 2017 at 2:17 AM, Paul Eggert <address@hidden> wrote:
>> On 02/10/2017 10:25 AM, Vibhav Pant wrote:
>>>
>>> Are there any other issues before I merge this into master?
>>
>>
>> For the C code, please use the usual style: space before paren in calls, GNU
>> style indenting for curly braces, "/* " at start of comments, main operator
>> like "||" at start of next line rather than at end of previous line.
>>
>> One of the 'if's is overparenthesized, i.e., "if ((E))" where E is an
>> ordinary expression and "if (E)" will do.
>>
>> Prefer "if (BYTE_CODE_SAFE)" to "#ifdef BYTE_CODE_SAFE", as these days it's
>> better to avoid the preprocessor when it's easy.
>>
>> This comment:
>>
>>                     /* Hash tables for switch are declared with :size set to
>> the
>>                        exact number of cases, thus
>>                        HASH_TABLE_SIZE (h) == h->count.  */
>>
>> is something that could be checked, no? Perhaps replace the comment with "if
>> (BYTE_CODE_SAFE) eassert (HASH_TABLE_SIZE (h) == h->count);" and do the
>> latter even with large hash tables?
> Done.
>>
>> If you change the loop from "for (i = 0; i < h->count; i++)" to "for (i =
>> h->count; 0 <= --i; )", then you can merge the two copies of "op = XINT
>> (HASH_VALUE (h, i)); goto ob_branch;" into one copy that is after the
>> surrounding "if".
>>
>
> Done, thanks.
>
>
> --
> Vibhav Pant
> address@hidden



-- 
Vibhav Pant
address@hidden