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

Remove special handling of box expressions from parser #120063

Merged
merged 2 commits into from
Jan 20, 2024

Conversation

clubby789
Copy link
Contributor

#108471 added a temporary hack to parse box expr. It's been almost a year since then, so I think it's safe to remove the special handling.

As a drive-by cleanup, move parser/removed-syntax* tests to their own directory.

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2024

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 17, 2024
@rust-cloud-vms rust-cloud-vms bot force-pushed the remove-box-handling branch from c41dfd0 to 3343f2f Compare January 17, 2024 16:49
@fmease
Copy link
Member

fmease commented Jan 17, 2024

Does this regress the following stable code (I can't check right now)?

macro_rules! check { (box $e:expr) => {} }
check! { box 0 }

I'm aware that you've reverted the change to ident_can_begin_expr, I just don't know what happens when there's a discrepancy between a can_begin_xxx and the parser.

@clubby789
Copy link
Contributor Author

No, that code compiles fine on the current reverted version

@Noratrieb
Copy link
Member

Hm, thinking about this a bit more lead to me to this summary of what this PR does: it replaced a really nice error with a worse error, while removing some complexity from the expression parser. I don't think I can justify making this error (that will probably be hit by people!) worse without the substantial benefit of freeing up the keyword, which can't happen anyways for now.
I'd prefer replacing the recovery with just an error expression and keep the nice error.

@rust-cloud-vms rust-cloud-vms bot force-pushed the remove-box-handling branch from 3343f2f to 5aea75d Compare January 17, 2024 21:19
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the remove-box-handling branch from 5aea75d to 3f7c784 Compare January 17, 2024 22:18
@compiler-errors
Copy link
Member

r? nilstrieb
pass back to me if you're too busy, though i trust your judgement above

@rustbot rustbot assigned Noratrieb and unassigned compiler-errors Jan 18, 2024
Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

thanks! this is a nice code simplicifaction

@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2024

📌 Commit 3f7c784 has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 20, 2024
…ilstrieb

Remove special handling of `box` expressions from parser

rust-lang#108471 added a temporary hack to parse `box expr`. It's been almost a year since then, so I think it's safe to remove the special handling.

As a drive-by cleanup, move `parser/removed-syntax*` tests to their own directory.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#118257 (Make traits / trait methods detected by the dead code lint)
 - rust-lang#119997 (Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias)
 - rust-lang#120000 (Ensure `callee_id`s are body owners)
 - rust-lang#120015 (coverage: Format all coverage tests with `rustfmt`)
 - rust-lang#120063 (Remove special handling of `box` expressions from parser)
 - rust-lang#120138 (Increase vscode settings.json `git.detectSubmodulesLimit`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#119997 (Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias)
 - rust-lang#120000 (Ensure `callee_id`s are body owners)
 - rust-lang#120063 (Remove special handling of `box` expressions from parser)
 - rust-lang#120116 (Remove alignment-changing in-place collect)
 - rust-lang#120138 (Increase vscode settings.json `git.detectSubmodulesLimit`)
 - rust-lang#120169 (Spelling fix)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8f5f967 into rust-lang:master Jan 20, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
Rollup merge of rust-lang#120063 - clubby789:remove-box-handling, r=Nilstrieb

Remove special handling of `box` expressions from parser

rust-lang#108471 added a temporary hack to parse `box expr`. It's been almost a year since then, so I think it's safe to remove the special handling.

As a drive-by cleanup, move `parser/removed-syntax*` tests to their own directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants