Improve parser#125117

oriongonza · 2024-05-14T11:21:16Z

Add a few more help diagnostics to incorrect semicolons
Overall improved that function
Addded a few comments
Renamed diff_marker fns to git_diff_marker

rustbot · 2024-05-14T11:21:23Z

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

wesleywiser · 2024-05-14T15:31:09Z

Nice improvements!

@bors r+ rollup

bors · 2024-05-14T15:31:12Z

📌 Commit bee186b has been approved by wesleywiser

It is now in the queue for this repository.

compiler/rustc_parse/src/parser/diagnostics.rs

compiler/rustc_parse/src/parser/item.rs

fmease · 2024-05-14T15:45:21Z

compiler/rustc_parse/src/parser/diagnostics.rs


-    fn diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option<Span> {
-        if self.is_diff_marker(long_kind, short_kind) {
+    fn git_diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option<Span> {


If I remember correctly it was intentional to not mention git here. I'd need to look for the original PRs. CC @estebank

Indeed, diff markers are used by git, but not exclusive to it.

fmease · 2024-05-14T15:46:03Z

Sorry but the markdown docs are butchered.
@bors r-

oriongonza · 2024-05-14T16:13:58Z

Thanks for noticing!

@rustbot ready

fmease · 2024-05-14T16:34:23Z

Thanks. One last thing, could you squash the commits into one?

oriongonza · 2024-05-14T16:42:16Z

All of them or just the "applied suggestions" one? If all with what name?

fmease · 2024-05-14T16:55:55Z

Eh, I guess git-fixup the “apply suggs” commit into 9db789a should be fine

oriongonza · 2024-05-14T21:09:25Z

@rustbot ready
About the git marker, what would be the reason not to include it in the name?

fmease · 2024-05-15T12:51:07Z

About the git marker, what would be the reason not to include it in the name?

As far as I'm aware, conflict markers (the >>>, ===, |||, <<<) are not specific to git but I'm sure about the specifics. Other VCSs also use them like SVN and Mercurial. I could be wrong but GNU's diff3 (three-way "merges") also produce them, not sure about GNU's diff (both from diffutils).

I think now might be the best time to rename [git] diff marker to conflict markers which is the more appropriate term as touched upon in #113826. If you could do that in this PR, that would be great!

Diff markers are +++, ---, @@, +, - and they come from the patch file format.

oriongonza · 2024-05-15T14:04:57Z

Maybe vcs_conflict_marker then?

fmease · 2024-05-15T20:00:31Z

Maybe vcs_conflict_marker then?

I guess that's fine, yes. From a purist / theoretical point of view, these conflict markers not necessarily tied to VCSs but to be fair, in practice, that shouldn't matter. CC https://www.gnu.org/software/diffutils/, https://www.geeksforgeeks.org/diff3-command-in-linux-with-examples/ (^^')

conflicts specifically and a few more improvements.

oriongonza · 2024-05-17T13:51:19Z

@rustbot ready
Ended up going for vcs_conflict_marker for all pub methods to make sure that whoever is reading that knows outright what it's referring to.

Question about the review: I rebased edit the specific commit with the fix. Should I have made the fix commit and made the fixup rebase after you approve so that it's easier to review?

fmease

Those a very nice changes, thanks a lot for your patience! I've noticed two small things but they're not worth blocking on. That can be done when fixing #113826, maybe you're up for it? ;).

fmease · 2024-05-18T00:22:02Z

compiler/rustc_parse/src/parser/diagnostics.rs

+    /// * `>>>>>`
+    /// * `=====`
+    /// * `<<<<<`
+    ///


Suggested change

/// * `>>>>>`

/// * `=====`

/// * `<<<<<`

///

/// * `>>>>>`

/// * `|||||`

/// * `=====`

/// * `<<<<<`

fmease · 2024-05-18T00:22:33Z

compiler/rustc_parse/src/parser/stmt.rs

            }
-            if self.is_diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
+            if self.is_vcs_conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
                // Account for `<<<<<<<` diff markers. We can't proactively error here because


Suggested change

// Account for `<<<<<<<` diff markers. We can't proactively error here because

// Account for `<<<<<<<` conflict markers. We can't proactively error here because

fmease · 2024-05-18T00:27:57Z

@bors r=wesleywiser,fmease rollup

bors · 2024-05-18T00:27:59Z

📌 Commit f8433a8 has been approved by wesleywiser,fmease

It is now in the queue for this repository.

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#125117 (Improve parser) - rust-lang#125184 (Fix ICE in non-operand `aggregate_raw_ptr` intrinsic codegen) - rust-lang#125240 (Temporarily revert to NonZeroUsize in rustc-abi to fix building on stable) - rust-lang#125248 (Migrate `run-make/rustdoc-scrape-examples-invalid-expr` to `rmake.rs`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#125117 - dev-ardi:improve-parser, r=wesleywiser,fmease Improve parser Fixes rust-lang#124935. - Add a few more help diagnostics to incorrect semicolons - Overall improved that function - Addded a few comments - Renamed diff_marker fns to git_diff_marker

Fix parsing of erroneously placed semicolons This closes rust-lang#124935, is a continuation of rust-lang#125245 after rebasing rust-lang#125117. Thanks `@gurry` for your code and sorry for making it confusing :P r? fmease

Fix parsing of erroneously placed semicolons This closes rust-lang#124935, is a continuation of rust-lang#125245 after rebasing rust-lang#125117. Thanks ``@gurry`` for your code and sorry for making it confusing :P r? fmease

Fix parsing of erroneously placed semicolons This closes rust-lang#124935, is a continuation of rust-lang#125245 after rebasing rust-lang#125117. Thanks ```@gurry``` for your code and sorry for making it confusing :P r? fmease

Rollup merge of rust-lang#125276 - dev-ardi:no-main-diag, r=fmease Fix parsing of erroneously placed semicolons This closes rust-lang#124935, is a continuation of rust-lang#125245 after rebasing rust-lang#125117. Thanks ```@gurry``` for your code and sorry for making it confusing :P r? fmease

rustbot assigned wesleywiser May 14, 2024

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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 14, 2024

wesleywiser approved these changes May 14, 2024

View reviewed changes

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 May 14, 2024

fmease requested changes May 14, 2024

View reviewed changes

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 14, 2024

rustbot 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 May 14, 2024

fmease 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 May 14, 2024

improve maybe_consume_incorrect_semicolon

8dc6a5d

oriongonza force-pushed the improve-parser branch from 444d380 to f1b404c Compare May 14, 2024 21:08

rustbot 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 May 14, 2024

fmease self-assigned this May 15, 2024

fmease 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 May 15, 2024

oriongonza added 3 commits May 17, 2024 15:45

Clarify that the diff_marker is talking about version control system

1f6d271

conflicts specifically and a few more improvements.

s/(Ident, ItemKind)/ItemInfo/

db5a616

use signature name for arg

f8433a8

oriongonza force-pushed the improve-parser branch from f1b404c to f8433a8 Compare May 17, 2024 13:47

rustbot 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 May 17, 2024

fmease approved these changes May 18, 2024

View reviewed changes

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 May 18, 2024

fmease mentioned this pull request May 18, 2024

Fix parsing of erroneously placed semicolons #125245

Closed

matthiaskrgr mentioned this pull request May 18, 2024

Rollup of 4 pull requests #125254

Merged

This comment was marked as outdated.

Sign in to view

bors merged commit f9bf759 into rust-lang:master May 18, 2024

rustbot added this to the 1.80.0 milestone May 18, 2024

oriongonza deleted the improve-parser branch May 19, 2024 12:32

oriongonza mentioned this pull request May 19, 2024

Fix parsing of erroneously placed semicolons #125276

Merged

	// Account for `<<<<<<<` diff markers. We can't proactively error here because
	// Account for `<<<<<<<` conflict markers. We can't proactively error here because

Uh oh!

Comments

Conversation

oriongonza commented May 14, 2024 • edited by fmease Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 14, 2024

Uh oh!

wesleywiser commented May 14, 2024

Uh oh!

bors commented May 14, 2024

Uh oh!

Uh oh!

Uh oh!

fmease May 14, 2024

Choose a reason for hiding this comment

Uh oh!

estebank May 17, 2024

Choose a reason for hiding this comment

Uh oh!

fmease commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oriongonza commented May 14, 2024

Uh oh!

fmease commented May 14, 2024

Uh oh!

oriongonza commented May 14, 2024

Uh oh!

fmease commented May 14, 2024

Uh oh!

oriongonza commented May 14, 2024

Uh oh!

fmease commented May 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oriongonza commented May 15, 2024

Uh oh!

fmease commented May 15, 2024

Uh oh!

oriongonza commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmease left a comment

Choose a reason for hiding this comment

Uh oh!

fmease May 18, 2024

Choose a reason for hiding this comment

Uh oh!

fmease May 18, 2024

Choose a reason for hiding this comment

Uh oh!

fmease commented May 18, 2024

Uh oh!

bors commented May 18, 2024

Uh oh!

This comment was marked as outdated.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

oriongonza commented May 14, 2024 •

edited by fmease

Loading

fmease commented May 14, 2024 •

edited

Loading

fmease commented May 15, 2024 •

edited

Loading

oriongonza commented May 17, 2024 •

edited

Loading