-
Notifications
You must be signed in to change notification settings - Fork 688
Parser style fixes after Class part II patch. #2574
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1286,14 +1286,10 @@ parser_parse_unary_expression (parser_context_t *context_p, /**< context */ | |
| if (PARSER_IS_CLASS_CONSTRUCTOR_SUPER (context_p->status_flags)) | ||
| { | ||
| parser_emit_cbc_ext (context_p, CBC_EXT_PUSH_CONSTRUCTOR_THIS); | ||
| } | ||
| else | ||
| { | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
| parser_emit_cbc (context_p, CBC_PUSH_THIS); | ||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
| break; | ||
| } | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
| parser_emit_cbc (context_p, CBC_PUSH_THIS); | ||
| break; | ||
| } | ||
| case LEXER_LIT_TRUE: | ||
|
|
@@ -1320,7 +1316,7 @@ parser_parse_unary_expression (parser_context_t *context_p, /**< context */ | |
| case LEXER_KEYW_SUPER: | ||
| { | ||
| if ((lexer_check_next_character (context_p, LIT_CHAR_DOT) | ||
| || lexer_check_next_character (context_p, LIT_CHAR_LEFT_SQUARE)) | ||
| || lexer_check_next_character (context_p, LIT_CHAR_LEFT_SQUARE)) | ||
| && context_p->status_flags & (PARSER_CLASS_HAS_SUPER | PARSER_IS_ARROW_FUNCTION)) | ||
| { | ||
| if (!LEXER_IS_BINARY_OP_TOKEN (context_p->stack_top_uint8)) | ||
|
|
@@ -1375,6 +1371,24 @@ parser_parse_unary_expression (parser_context_t *context_p, /**< context */ | |
| lexer_next_token (context_p); | ||
| } /* parser_parse_unary_expression */ | ||
|
|
||
| /** | ||
| * Push an eval opcode. | ||
| */ | ||
| static inline void | ||
| parser_emit_eval (parser_context_t *context_p) /**< context */ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this function OK here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there is such rule. This is a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I was not referring to rules as such but to coding patterns, especially related to |
||
| { | ||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
| if (context_p->status_flags & PARSER_CLASS_HAS_SUPER) | ||
| { | ||
| uint8_t data_byte = (uint8_t) PARSER_GET_CLASS_ECMA_PARSE_OPTS (context_p->status_flags); | ||
| parser_emit_cbc_ext_byte (context_p, CBC_EXT_CLASS_EVAL, data_byte); | ||
| return; | ||
| } | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
|
|
||
| parser_emit_cbc (context_p, CBC_EVAL); | ||
| } /* parser_emit_eval */ | ||
|
|
||
| /** | ||
| * Parse the postfix part of unary operators, and | ||
| * generate byte code for the whole expression. | ||
|
|
@@ -1557,20 +1571,7 @@ parser_process_unary_expression (parser_context_t *context_p) /**< context */ | |
|
|
||
| if (is_eval) | ||
| { | ||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
| if (context_p->status_flags & PARSER_CLASS_HAS_SUPER) | ||
| { | ||
| parser_flush_cbc (context_p); | ||
| context_p->last_cbc_opcode = PARSER_TO_EXT_OPCODE (CBC_EXT_CLASS_EVAL); | ||
| context_p->last_cbc.value = PARSER_GET_CLASS_ECMA_PARSE_OPTS (context_p->status_flags); | ||
| } | ||
| else | ||
| { | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
| parser_emit_cbc (context_p, CBC_EVAL); | ||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
| } | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
| parser_emit_eval (context_p); | ||
| } | ||
|
|
||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
|
|
@@ -1813,13 +1814,14 @@ parser_append_binary_token (parser_context_t *context_p) /**< context */ | |
| parser_stack_push_uint16 (context_p, context_p->last_cbc.literal_index); | ||
| parser_stack_push_uint8 (context_p, CBC_ASSIGN_PROP_LITERAL); | ||
| context_p->last_cbc_opcode = PARSER_CBC_UNAVAILABLE; | ||
|
|
||
| #ifndef CONFIG_DISABLE_ES2015_CLASS | ||
| if (context_p->status_flags & PARSER_CLASS_SUPER_PROP_REFERENCE) | ||
| { | ||
| context_p->status_flags &= (uint32_t) ~PARSER_CLASS_SUPER_PROP_REFERENCE; | ||
| parser_emit_cbc_ext (context_p, CBC_EXT_SUPER_PROP_ASSIGN); | ||
| parser_flush_cbc (context_p); | ||
| } | ||
| context_p->status_flags &= (uint32_t) ~PARSER_CLASS_SUPER_PROP_REFERENCE; | ||
| #endif /* !CONFIG_DISABLE_ES2015_CLASS */ | ||
| } | ||
| else | ||
|
|
||
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.
Please, add these back. It helps in various cases to have comma-terminated arrays (and enums). If an array (or an enum) has to me changed (new element gets added, an old one gets removed) then only that line needs to be changed that contains the affected element. One doesn't have to check whether it is/was/becomes before/after/the last element in the array (or enum) and doesn't have to add/remove another comma. It also helps if content is ifdef-guarded: again, one doesn't have to care about which element is the last. So, please keep also the last entry comma-terminated.
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 think such change is rare because the items are in alphabetic order. And we usually don't put a comma after the last item in other enums as well.
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.
These are the arrays and enums where the use-trailing-comma-everywhere principle is used:
jerry_typedarray_mappings, ecma_builtin_routines, ecma_builtin_call_functions, ecma_builtin_construct_functions, ecma_builtin_property_list_references, ecma_error_mappings, cbc_flags, cbc_ext_flags, cbc_names, cbc_ext_names, statement_lengths
jerry_debugger_flags_t, jerry_debugger_header_type_t, jerry_debugger_configuration_flags_t, jerry_debugger_eval_type_t, jerry_debugger_eval_result_type_t, jerry_debugger_output_subtype_t, ecma_init_flag_t, ecma_status_flag_t, ecma_parse_opts_t, ecma_property_types_t, ecma_list_properties_options_t, ecma_property_flags_t, ecma_collection_flag_t, ecma_direct_string_type_t, ecma_arraybuffer_extra_flag_t, ecma_property_hashmap_delete_status, ecma_builtin_property_type_t, ecma_builtin_number_type_t, ecma_array_object_set_length_flags_t, jerry_init_flag_t, jerry_type_t, jerry_typedarray_type_t, jerry_debugger_wait_for_source_status_t, jerry_generate_snapshot_opts_t, jerry_exec_snapshot_opts_t, jmem_free_unused_memory_severity_t, cbc_code_flags, cbc_opcode_t, cbc_ext_opcode_t, lexer_literal_type_t, lexer_literal_status_flags_t, skip_mode_t, lexer_token_type_t, lexer_obj_ident_opts_t, lexer_literal_object_type_t, lexer_number_type_t, parser_object_literal_item_types_t, parser_general_flags_t, parser_expression_flags_t, scan_modes_t, scan_stack_modes_t, parser_statement_type_t, parser_try_block_type_t, re_token_type_t, number_arithmetic_op, number_bitwise_logic_op, vm_stack_context_type_t, vm_oc_get_types, vm_oc_types, vm_oc_put_types, vm_call_operation
That's quite some, I think. But not universal, for sure. The style is mixed in the code base. But if I had to choose between future proof and not future proof, I'd go for the first option. And definitely would not change back from the first style to the latter one.