-
Notifications
You must be signed in to change notification settings - Fork 687
Introduce parser statement flags #3318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| /** | ||
| * Parser statement attributes. | ||
| * Note: the order of the attributes must be keep in sync with parser_statement_type_t | ||
| */ | ||
| static const uint8_t parser_statement_flags[] = | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea to remove redundancies and simplify the code base. But it would be great if we could make sure that the enum and this array are in synch.
If we can change the order of enum values, we can do it with macros, for example:
#define PARSER_STATEMENT_TYPE(F) \
F(PARSER_STATEMENT_START, PARSER_STATM_NO_OPTS), \
F(PARSER_STATEMENT_BLOCK, PARSER_STATM_NO_OPTS), \
...
#define PARSER_STATEMENT_TYPE_ES2015(F) ...
#define EXPAND1(A, B) A
#define EXPAND2(A, B) B
typedef enum
{
PARSER_STATEMENT_TYPE(EXPAND1)
#if ENABLED (JERRY_ES2015)
PARSER_STATEMENT_TYPE_ES2015(EXPAND1)
#endif /* ENABLED (JERRY_ES2015) */
} parser_statement_type_t;
static const uint8_t parser_statement_flags[] =
{
PARSER_STATEMENT_TYPE(EXPAND2)
#if ENABLED (JERRY_ES2015)
PARSER_STATEMENT_TYPE_ES2015(EXPAND2)
#endif /* ENABLED (JERRY_ES2015) */
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, probably it can be combined with statement_lengths[] as well.
@zherczeg What do you think about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate patch. I want to se how it looks like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(lines in the length are quite long, and we have 120 char limit)
These flags makes the code more readable also makes the process of introducing a new statement type easier. JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]
422b30e to
4ac694a
Compare
zherczeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dbatyai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
These flags makes the code more readable also makes the process of introducing a new statement type easier.
JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]