Replace multiple boolean arguments to one uint8_t. #1100
Conversation
|
Binary size decreased by 1K. |
| bool is_writable, /**< 'Writable' attribute */ | ||
| bool is_enumerable, /**< 'Enumerable' attribute */ | ||
| bool is_configurable) /**< 'Configurable' attribute */ | ||
| uint8_t prop_attributes) /**< property attributes */ |
There was a problem hiding this comment.
I like this idea, but I'd give a name to this. What do you think about typedef uint8_t prop_attr_t; or typedef uint8_t prop_attr_flag_t;?
There was a problem hiding this comment.
I just saw that we already have a ecma_property_flags_t, but it is an enum, so it'll be 32bit long (usually, but not always). In this case I think it is unnecessary to create a new type, but it should be mentioned in the comment.
There was a problem hiding this comment.
That is a flagset, not a flag. The type_and_flags in ecma_property_t is also an uint8_t. I wouldn't want to change it.
There was a problem hiding this comment.
As I said, you don't have to change the name, just make a comment.
...
uint8_t prop_attributes) /**< property attributes (See: ecma_property_flags_t) */
|
Any more thoughts? |
zherczeg
force-pushed
the
property_create_optimization
branch
from
a396b0f to
73807c6
Compare
|
LGTM, after the comment fixed. |
the three boolean arguments of ecma_create_named_data_property and the two boolean arguments of ecma_create_named_accessor_property are combined into one uint8_t argument. On ARM-32 it is preferred to have less than four arguments, since these arguments can be passed in registers. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
zherczeg
force-pushed
the
property_create_optimization
branch
from
73807c6 to
8c92972
Compare
|
LGTM |
No description provided.