Bug 46359 – Message autogeneration script should parse sync message syntax

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Message autogeneration script should parse sync message syntax

[Anders Carlsson]

Created attachment 68526 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 68526 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68526&action=review

> WebKit2/Scripts/webkit2/messages.py:87
> +                    reply_parameters = [Parameter(*type_and_name.split(' ')) for type_and_name in reply_parameters_string.split(', ')]

Maybe we should add a function that does this instead of duplicating the code.

> WebKit2/Scripts/webkit2/messages.py:102
> +        if self.reply_parameters != None:

I think "is not" is preferred over "!=".

> WebKit2/Scripts/webkit2/messages.py:136
> +    builtin_types = ['bool', 'uint8_t', 'uint16_t', 'uint32_t', 'uint64_t']

Wrapping this in set() would be good.

I don't see a test that passes a type with :: as a non-reference.

> WebKit2/Scripts/webkit2/messages.py:166
> +def reply_base_class(message):
> +    reply_base_class = 'CoreIPC::Arguments%d' % len(message.reply_parameters)
> +    if len(message.reply_parameters):
> +        reply_base_class = '%s<%s>' % (reply_base_class, ', '.join(reply_parameter_type(parameter.type) for parameter in message.reply_parameters))
> +    return reply_base_class
> +
> +
> +def delayed_reply_base_class(message):
> +    delayed_reply_base_class = 'CoreIPC::Arguments%d' % len(message.reply_parameters)
> +    if len(message.reply_parameters):
> +        delayed_reply_base_class = '%s<%s>' % (delayed_reply_base_class, ', '.join(function_parameter_type(parameter.type) for parameter in message.reply_parameters))
> +    return delayed_reply_base_class

So much code duplication! Can we just pass the *_parameter_type function as a parameter?

> WebKit2/Scripts/webkit2/messages_unittest.py:154
> +        if message.reply_parameters != None:
> +            for index, parameter in enumerate(message.reply_parameters):
> +                self.assertEquals(parameter.type, expected_message['reply_parameters'][index][0])
> +                self.assertEquals(parameter.name, expected_message['reply_parameters'][index][1])

Hm, this won't catch cases where we expect there to be reply parameters but for some reason the parsing screwed up and found none.

> WebKit2/Scripts/webkit2/messages_unittest.py:173
> +            if message.reply_parameters != None:
> +                self.assertEquals(messages.reply_base_class(message), _expected_results['messages'][index]['reply_base_class'])
> +                if message.delayed:
> +                    self.assertEquals(messages.delayed_reply_base_class(message), _expected_results['messages'][index]['delayed_reply_base_class'])

Ditto.

[Anders Carlsson]

Committed r68153: <http://trac.webkit.org/changeset/68153>