Skip to content
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 made to ensure that '=>' token is parsed properly #3292

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sriganeshres
Copy link
Contributor

@sriganeshres sriganeshres commented Dec 4, 2024

gcc/rust/ChangeLog:

* parse/rust-parse-impl.h (Parser::parse_expr): Updated parsing logic to stop on
encountering '=>' or ',' while having restrictions.stop_on_token turned on.
(Parser::left_denotations): Updated to stop parsing on the special tokens mentioned above.
(Parser::null_denotation): Updated to stop parsing on the special tokens mentioned above.
* parse/rust-parse.h (struct ParseRestrictions): Added a field stop_on_token (default false)
to maintain the above functionality.

Fixes #3099
Here is a checklist to help you with your PR.

  • I have made changes to 3 functions Parser::parse_expr, Parser::left_denotations, Parser::null_denotation So that they stop on special token like => and ,. They would terminate the parsing to allow exatract expression.
  • Also I have added a test to verify the running of this.

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

Thanks @sriganeshres for contributing!! Could you add a testcase to your PR to ensure we don't introduce a regression on this in the future?

@CohenArthur CohenArthur self-assigned this Dec 4, 2024
@sriganeshres sriganeshres force-pushed the chore-parse-expr branch 2 times, most recently from b86ad1a to 62abc58 Compare December 4, 2024 18:33
@sriganeshres
Copy link
Contributor Author

Thanks @sriganeshres for contributing!! Could you add a testcase to your PR to ensure we don't introduce a regression on this in the future?

Working on that. I have ran make check-rust it gave correct results. Can you give me an idea of what type of test to be written for this.

Thank You.

@sriganeshres sriganeshres force-pushed the chore-parse-expr branch 7 times, most recently from 5aca0e5 to 7897d6e Compare December 5, 2024 14:12
gcc/rust/ChangeLog:

	* parse/rust-parse-impl.h (Parser::parse_expr): Updated parsing logic to stop on
	encountering '=>' or ',' while having restrictions.stop_on_token turned on.
	(Parser::left_denotations): Updated to stop parsing on the special tokens mentioned above.
	(Parser::null_denotation): Updated to stop parsing on the special tokens mentioned above.
	* parse/rust-parse.h (struct ParseRestrictions): Added a field stop_on_token (default false)
	to maintain the above functionality.
@P-E-P
Copy link
Member

P-E-P commented Dec 9, 2024

Working on that. I have ran make check-rust it gave correct results. Can you give me an idea of what type of test to be written for this.

You could write a test from the snippet given in the fixed issue, it would pass with your changes

	* rust/compile/issue-3099.rs: Added a new test for issue-3099 to parse '=>' properly.
@@ -89,6 +89,8 @@ struct ParseRestrictions
bool entered_from_unary = false;
bool expr_can_be_null = false;
bool expr_can_be_stmt = false;
bool stop_on_token
= false; // This is for stopping parsing at a specific token
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? It seems like a hack

Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

This doesnt seem correct to me the stop_on_token doesn't seem to be set to true anywhere so to me this doesn't seem to solve anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

parse_expr not stopping on =>
4 participants