Skip to content

Syntax error feedback support #211

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jun 19, 2015
Merged

Syntax error feedback support #211

merged 6 commits into from
Jun 19, 2015

Conversation

ruben-ayrapetyan
Copy link
Contributor

Related issue: #54

@ruben-ayrapetyan ruben-ayrapetyan added normal parser Related to the JavaScript parser development Feature implementation labels Jun 18, 2015
@ruben-ayrapetyan ruben-ayrapetyan added this to the Core ECMA features milestone Jun 18, 2015
@ruben-ayrapetyan
Copy link
Contributor Author

@sand1k, could you, please, review the changes?

@sand1k
Copy link
Contributor

sand1k commented Jun 18, 2015

@ruben-ayrapetyan, why do we need to move jerry-core/parser/collections to jerry-core/parser/js/collections?
You mean that 'parser' folder is going to contain other parsers?

@sand1k
Copy link
Contributor

sand1k commented Jun 18, 2015

As for the rest, LGTM.

@ruben-ayrapetyan
Copy link
Contributor Author

@ruben-ayrapetyan, why do we need to move jerry-core/parser/collections to jerry-core/parser/js/collections?
You mean that 'parser' folder is going to contain other parsers?

@sand1k, thank you for review.

The collections are currently used only in js parser. As one of the commit in the changes replaces heap allocation requests from the collections component with requests to parser-internal memory manager, it seems to me that it is better to move the collections component to make them parser-internal.

Currently there are following collection types:

  • linked_list:
  • hash_table:
    • used for conversion of literal identifiers to idx_t for a specific opcode block // maybe, would also be removed in context of Implement byte-code storage #46
  • parser stacks // seems that there are some places that are currently using the parser stacks, but could be reimplemented without using them, while reducing parser memory usage;
  • array_list:
    • parser stacks (maybe, would be removed if parser stacks would be removed, or reimplemented).

It seems to me that, probably, during further optimization of parser memory usage, all code, which is currently using the collections, could be updated to not use them.

What do you think about this?

@ruben-ayrapetyan ruben-ayrapetyan assigned sand1k and unassigned egavrin Jun 18, 2015
@sand1k
Copy link
Contributor

sand1k commented Jun 19, 2015

@ruben-ayrapetyan, sounds reasonoble. Do we need 'js' folder, which is now the only one inside 'parser'?

@egavrin
Copy link
Contributor

egavrin commented Jun 19, 2015

@ruben-ayrapetyan, sounds reasonoble. Do we need 'js' folder, which is now the only one inside 'parser'?

No, I think

@ruben-ayrapetyan
Copy link
Contributor Author

@ruben-ayrapetyan, sounds reasonoble. Do we need 'js' folder, which is now the only one inside 'parser'?

@sand1k, @egavrin,
The #169 pull request adds parser/regexp folder.
So, may be the js folder is still necessary.

@ruben-ayrapetyan ruben-ayrapetyan assigned galpeter and unassigned sand1k Jun 19, 2015
galpeter
galpeter reviewed Jun 19, 2015
View reviewed changes
jerry-core/parser/js/syntax-errors.cpp
* syntax_get_syntax_error_longjmp_label
* syntax_raise_error
*/
jmp_buf jsp_syntax_error_label;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a static variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is anyway inaccessible outside of the module, as there are no extern declarations for it. I could make it static, if you prefer so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better as the other variables here are also static (the STATIC_STACKs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll add it.

@egavrin
Copy link
Contributor

egavrin commented Jun 19, 2015

@ruben-ayrapetyan I briefly checked, good to me. Get approve from @galpeter and make push

@ruben-ayrapetyan ruben-ayrapetyan force-pushed the Ruben-syntax-error-feedback branch from f8dff65 to fe50a02 Compare June 19, 2015 12:58
@ruben-ayrapetyan
Copy link
Contributor Author

@galpeter, I've updated pull request:

  • added unit test for parse of var var;
  • updated * alignment.

Please, add your note to discussion about static specifier.

@ruben-ayrapetyan ruben-ayrapetyan force-pushed the Ruben-syntax-error-feedback branch from fe50a02 to aa8f242 Compare June 19, 2015 13:22
@ruben-ayrapetyan
Copy link
Contributor Author

@galpeter, I've updated pull request.

@galpeter
Copy link
Contributor

@ruben-ayrapetyan, thanks. Looks good to me.

@galpeter galpeter removed their assignment Jun 19, 2015
@ruben-ayrapetyan
Copy link
Contributor Author

@galpeter, thank you for review.

@ruben-ayrapetyan
Move jerry-core/parser/collections to jerry-core/parser/js/collections.
2bf25f1 
JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan
Remove parser_init / parser_free interfaces (corresponding actions ar…
4e56393 
…e now performed in parser_parse_program); introduce boolean return value in parser invocation interfaces that would indicate whether SyntaxError was raised during parse.

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan
Improve allocation of memory for byte-code array and literal identifi…
bd9d1d4 
…ers hash table.

 - literal identifiers hash table and byte-code array are now allocated in one heap block instead of four blocks;
 - memory regions with the hash tables and arrays are now linked into linked list using headers at start of the regions instead of using array_list.

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan
Introduce jsp_label_remove_all_labels interface that removes all jump…
9b0f5d4 
… labels registered at the moment of call for usage upon raise of SyntaxError during parse.

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan
Introduce parser-specific managed memory allocator with interface jsp…
85f12de 
…_mm_free_all that, upon call, frees all memory allocated with the allocator.

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan ruben-ayrapetyan force-pushed the Ruben-syntax-error-feedback branch from aa8f242 to 6fe2358 Compare June 19, 2015 13:25
@ruben-ayrapetyan
Support syntax error feedback in parser.
a4e54e7 
Now, parser correctly finishes parse procedure if syntax of source code is incorrect (syntax correctness is indicated using return value):
 - parser-internal memory management is performed using jsp_mm_alloc / jsp_mm_free;
 - upon detection of incorrect syntax, all parser-allocated memory regions are deallocated using jsp_mm_free_all and parse finishes with corresponding return value.

JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan r.ayrapetyan@samsung.com
@ruben-ayrapetyan ruben-ayrapetyan force-pushed the Ruben-syntax-error-feedback branch from 6fe2358 to a4e54e7 Compare June 19, 2015 14:05
@ruben-ayrapetyan ruben-ayrapetyan merged commit a4e54e7 into master Jun 19, 2015
@ruben-ayrapetyan ruben-ayrapetyan deleted the Ruben-syntax-error-feedback branch June 19, 2015 15:00
This was referenced May 17, 2020
Closed
Closed
This was referenced Oct 20, 2020
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
development Feature implementation normal parser Related to the JavaScript parser
Projects
None yet
Milestone
Core ECMA features
Development

Successfully merging this pull request may close these issues.
4 participants
@ruben-ayrapetyan @sand1k @egavrin @galpeter