-
Notifications
You must be signed in to change notification settings - Fork 688
Change literal status flags to enum. #2474
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
|
|
||
| /** | ||
| * Maximum number of values pushed onto the stack by a function. | ||
| * Maximum number of values pushed onto the stack by a function. |
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.
There is a 'C2 A0' character here, which a "NO-BREAK SPACE'. Change it to a normal space.
jerry-core/parser/js/common.h
Outdated
| */ | ||
| typedef enum | ||
| { | ||
| LEXER_FLAG_VAR = (1 << 0), /** local identifier (var, function arg) */ |
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.
All the enum value comments need to be /**< as they are after the entity they are documenting.
|
Thank you for the review. Patch updated. |
jerry-core/parser/js/common.h
Outdated
| LEXER_FLAG_SOURCE_PTR = (1 << 6), /**< the literal is directly referenced in the source code | ||
| * (no need to allocate memory) */ | ||
| LEXER_FLAG_LATE_INIT = (1 << 7), /**< initialize this variable after the byte code is freed */ | ||
| } parser_lexer_literal_status_flags_t; |
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.
Strange name for this type. It is used in lexer_literal_t, all the enum values have a LEXER_ prefix, yet this type has a parser_ prefix. I'd suggest going for lexer_literal_status_flags_t only.
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.
Good catch. I haven't noticed that.
Also fix a white space typo. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
akosthekiss
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
|
LGTM (informal) |
LaszloLango
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
* Revert "jerry: ref to function self should not create new object (#368)" * jerry: change literal status flags to enum Picked from jerryscript-project/jerryscript#2474 * jerry: fix named function expression creation picked from jerryscript-project/jerryscript#2634 * Include test sets
* Revert "jerry: ref to function self should not create new object (#368)" * jerry: change literal status flags to enum Picked from jerryscript-project/jerryscript#2474 * jerry: fix named function expression creation picked from jerryscript-project/jerryscript#2634 * Include test sets
* Revert "jerry: ref to function self should not create new object (#368)" * jerry: change literal status flags to enum Picked from jerryscript-project/jerryscript#2474 * jerry: fix named function expression creation picked from jerryscript-project/jerryscript#2634 * Include test sets
Also fix a typo.