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.

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

When recovering from a : in a pattern, use adequate AST pattern #87160

Merged
merged 2 commits into from
Dec 1, 2021
Merged

When recovering from a : in a pattern, use adequate AST pattern #87160

merged 2 commits into from
Dec 1, 2021

Conversation

estebank
Copy link
Contributor

If the suggestion to use :: instead of : in the pattern isn't correct, a second resolution error will be emitted.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 15, 2021
@JohnCSimon JohnCSimon added A-allocators Area: Custom and system allocators S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed A-allocators Area: Custom and system allocators S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2021
@joelpalmer joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 17, 2021
@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 28, 2021
@estebank
Copy link
Contributor Author

estebank commented Oct 4, 2021

ping

@nagisa
Copy link
Member

nagisa commented Oct 7, 2021

r? @nagisa

@nagisa nagisa assigned nagisa and unassigned matthewjasper Oct 7, 2021
nagisa
nagisa reviewed Oct 7, 2021
View reviewed changes
Copy link
Member

@nagisa nagisa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think extracting the code to "join" two patterns into a function might make the code cleaner.

src/test/ui/parser/issue-87086-colon-path-sep.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated
}
PatKind::Path(old_qself, old_path) => {
for segment in old_path.segments.iter().rev() {
path.segments.insert(0, segment.clone());
Copy link
Member

Choose a reason for hiding this comment

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

old_path.segments.extend(path.segments) or something of the sort would be nicer, perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think of this?

                                                    path.segments = old_path
                                                        .segments
                                                        .iter()
                                                        .cloned()
                                                        .chain(take(&mut path.segments))
                                                        .collect();

@nagisa nagisa added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2021
@bors

This comment has been minimized.

Sign in to view
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 22, 2021
@bors
Copy link
Contributor

bors commented Nov 25, 2021

☔ The latest upstream changes (presumably #85346) made this pull request unmergeable. Please resolve the merge conflicts.

nagisa
nagisa reviewed Nov 27, 2021
View reviewed changes
compiler/rustc_parse/src/parser/diagnostics.rs Outdated
span,
"maybe write a path separator here",
"::".to_string(),
Applicability::MachineApplicable,
Copy link
Member

Choose a reason for hiding this comment

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

Is this truly MachineApplicable? We don't really check with the resolver here that the constructed pattern will match, do we?

nagisa
nagisa approved these changes Nov 27, 2021
View reviewed changes
Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

Seems good, except for machine applicability stuff.

@estebank
Copy link
Contributor Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Nov 29, 2021

📌 Commit c027105 has been approved by nagisa

@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 Nov 29, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 29, 2021
@matthiaskrgr
Rollup merge of rust-lang#87160 - estebank:colon-recovery, r=nagisa
0104842 
When recovering from a `:` in a pattern, use adequate AST pattern

If the suggestion to use `::` instead of `:` in the pattern isn't correct, a second resolution error will be emitted.
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 29, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 30, 2021
@matthiaskrgr
Rollup merge of rust-lang#87160 - estebank:colon-recovery, r=nagisa
aec1574 
When recovering from a `:` in a pattern, use adequate AST pattern

If the suggestion to use `::` instead of `:` in the pattern isn't correct, a second resolution error will be emitted.
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 30, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 1, 2021
@matthiaskrgr
Rollup merge of rust-lang#87160 - estebank:colon-recovery, r=nagisa
446d2dc 
When recovering from a `:` in a pattern, use adequate AST pattern

If the suggestion to use `::` instead of `:` in the pattern isn't correct, a second resolution error will be emitted.
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 1, 2021
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 1, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2021
@bors
Auto merge of rust-lang#91418 - matthiaskrgr:rollup-vn9f9w3, r=matthi…
26b4557 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#87160 (When recovering from a `:` in a pattern, use adequate AST pattern)
 - rust-lang#90985 (Use `get_diagnostic_name` more)
 - rust-lang#91087 (Remove all migrate.nll.stderr files)
 - rust-lang#91207 (Add support for LLVM coverage mapping format versions 5 and 6)
 - rust-lang#91298 (Improve error message for `E0659` if the source is not available)
 - rust-lang#91346 (Add `Option::inspect` and `Result::{inspect, inspect_err}`)
 - rust-lang#91404 (Fix bad `NodeId` limit checking.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e68e5d2 into rust-lang:master Dec 1, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 1, 2021
@estebank estebank deleted the colon-recovery branch November 9, 2023 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nagisa nagisa nagisa approved these changes

Assignees

@nagisa nagisa

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
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@estebank @rust-highfive @nagisa @bors @JohnCSimon @joelpalmer @matthewjasper @inquisitivecrystal @rustbot