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

[Vibhav Pant]

I'll push this to master after adding a few more tests for byte-switch
to test/lisp/emacs-lisp/bytecode-tests.el

On Sun, Feb 12, 2017 at 8:40 AM, Vibhav Pant <address@hidden> wrote:
> 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



-- 
Vibhav Pant
address@hidden