-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Corrects issue #28777 by removing, once a binary operator is found, the #30375
Conversation
… found, the RESTRICTION_STMT_EXPR restriction to allow subsequent expressions to contain braces. rust-lang#28777
r? @eddyb (random @rust-lang/compiler selection) |
Fixity::Right => self.with_res(restrictions, |this|{ | ||
this.parse_assoc_expr_with(op.precedence(), LhsExpr::NotYetParsed) | ||
Fixity::Right => self.with_res( | ||
restrictions & !Restrictions::RESTRICTION_STMT_EXPR, |
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.
This can be written as restrictions - Restrictions::RESTRICTION_STMT_EXPR
, AFAIK.
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.
You are correct. I missed the support for - as set difference in bitflags. Thank you.
Changed bit manipulation to use supported - (set difference) instead of explicit '& !'.
@bors r+ |
📌 Commit 41cc365 has been approved by |
RESTRICTION_STMT_EXPR restriction to allow subsequent expressions to contain braces. rust-lang#28777
I guess |
@bors: r- this failed the rollup |
Yes, parse-fail/assoc-oddities-3.rs fails, as noted in #28777, because it is successfully parsed after this modification which is then consistent with grammar/parser-lalr. Please excuse my ignorance relating to this process, but it is not clear to me if I should remove that test as part of my PR or wait for a confirmation that this is the desired behavior (is grammar/parser-lalr considered the model?). |
modification to parsing of binary operators. This is consistent with the behavior of grammar/parser-lalr.
Committed with test case removed. |
run-pass. Added run-pass/issue-28777 to demonstrate behavior of this parsing modification.
The original parse-fail/assoc-oddities-3.rs test has been updated and moved to run-pass. An additional test has been added to run-pass to demonstrate that this modification successfully addresses the issue (this should have been done in the first place). |
ping @eddyb r? EDIT: @aaronkeen ah, by the way, that test was written by me to ensure my changes to associative operator parsing do not change behaviour of the previous parser (since the changes have been quite extensive). It is (or was), as name suggests, an oddity. |
@bors r+ |
📌 Commit cedd794 has been approved by |
⌛ Testing commit cedd794 with merge 37ff5c8... |
💔 Test failed - auto-linux-64-opt |
It seems the following tests timed out. I have examined the tests in code snippets in primitive-types and do not see any code that should have been perturbed by this PR. These tests all passed on my local machine [x86_64-apple-darwin], so I am not sure what the issue is. Is it possible to retrieve information regarding the specific test that failed (the execution order differs from the file and from my machine)?
|
@aaronkeen I believe the failure is spurious and a few retries should make it go away. |
@bors: retry |
RESTRICTION_STMT_EXPR restriction to allow subsequent expressions to contain braces. #28777
RESTRICTION_STMT_EXPR restriction to allow subsequent expressions to
contain braces.
#28777