Recover on attribute in use tree by scrabsha · Pull Request #155254 · rust-lang/rust

scrabsha · 2026-04-13T19:06:05Z

Renders as:

This requires passing two more arguments to Parser::parse_use_tree and I'm not super happy about it. I did consider adding more context to Parser instead, but that did not sound like a good idea. Happy to rework my code to make it less ugly :)

I am also not very happy that the deletion shows up as a green ~ in the suggestion. I tried very hard to make it render properly but failed. I am not sure it's actually possible but would love to be proven wrong hehe

scrabsha · 2026-04-14T20:17:27Z

@fmease would you be ok with me assigning this PR to you? (once I have re-reviewed it again)

fmease · 2026-04-14T20:51:46Z

Sure! :)

scrabsha · 2026-04-17T15:48:00Z

+                .filter_map(|segment| {
+                    if !segment.ident.is_special() { Some(segment.ident.as_str()) } else { None }
+                })


This filter_map removes any segment that is a special identifier - not just the leading {{root}}. I cannot think of a case in which it is an issue, but I want to mention it regardless.

View changes since the review

Fair. However if you were to map kw::PathRoot to "" here, you wouldn't need global_path, right?

scrabsha · 2026-04-17T15:52:24Z

The changes in the file are needed because the closure passed to span_extend_while in item.rs mutably borrows comma_reached.

View changes since the review

scrabsha · 2026-04-17T15:53:28Z

thank you!

r? fmease

rustbot · 2026-04-17T15:53:31Z

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

rustbot · 2026-04-17T15:53:52Z

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

fmease

Preliminary review, sorry for the delay.

View changes since this review

fmease · 2026-05-03T13:26:17Z

    /// The tokens must be either a delimited token stream, or empty token stream,
    /// or the "legacy" key-value form.
+    ///
+    /// ```none


Suggested change

/// ```none

/// ```text

text is conventional for plain text.

fmease · 2026-05-03T13:33:22Z

    ///     PATH `(` TOKEN_STREAM `)`
    ///     PATH `[` TOKEN_STREAM `]`
    ///     PATH `{` TOKEN_STREAM `}`
    ///     PATH
    ///     PATH `=` UNSUFFIXED_LIT


Since this is now in a fenced code block, this should be dedented by 4 spaces.

The reason that these grammar rules were indented is because somebody tried to write an indented code block but didn't precede it by a blank line, so it didn't get interpreted that way.

fmease · 2026-05-22T11:21:08Z

+                .psess
+                .source_map()
+                .span_to_source(attr_span, |s, start, end| Ok(s[start..end].to_string()))
+                .unwrap();


span_to_source can fail, so please don't unwrap. It can return None if the span points into the expansion of a macro from an upstream crate whose sources aren't available for some reason.

See RUST-128442 and RUST-128445 for how an artificial reproducer might look like.

Instead suppress the suggestion if we hit None in any of those places where you call span_to_source.

fmease · 2026-05-22T13:14:58Z

+            let err = crate::errors::AttrInUseTree {
+                attr_span,
+                sub: Some(crate::errors::AttrInUseTreeSugg {
+                    use_lo: use_token_span.shrink_to_lo(),


(minor) This doesn't account for attributes on the use-item itself, the start index of the use keyword is not sufficient. I'm pretty sure your code currently suggests something like

#[deny(unused_imports)] #[allow(unused_imports)] use crate::fst; use crate::{ snd, };

for

#[deny(unused_imports)] use crate::{ #[allow(unused_imports)] fst, snd, };

doesn't need to be addressed now, can also be a FIXME.

View changes since the review

fmease

Some more comments but this already looks very good overall, thanks for working on this!

View changes since this review

fmease · 2026-05-22T13:43:23Z

+            if p.check_noexpect(&TokenKind::Pound)
+                && p.look_ahead(1, |tok| matches!(tok.kind, TokenKind::OpenBracket))


I would just parse_outer_attributes unconditionally, take for recovery and raise an error if the attr vec is non-empty. That'd be a lot simpler and it would allow us to recover from "leading" doc comments (we're already recovering from doc comments if they're preceded by regular attributes).

fmease · 2026-05-22T13:50:21Z

+            let use_tree = self
+                .psess
+                .source_map()
+                .span_to_source(use_tree_span, |s, start, end| Ok(s[start..end].to_string()))


Suggested change

.span_to_source(use_tree_span, |s, start, end| Ok(s[start..end].to_string()))

.span_to_snippet(use_tree_span)

fmease · 2026-05-22T13:53:07Z

+            let attr = self
+                .psess
+                .source_map()
+                .span_to_source(attr_span, |s, start, end| Ok(s[start..end].to_string()))


Suggested change

.span_to_source(attr_span, |s, start, end| Ok(s[start..end].to_string()))

.span_to_snippet(attr_span)

fmease · 2026-05-22T14:02:42Z

 }
+
+struct UsePathList<'a> {
+    element: &'a [ast::PathSegment],


Suggested change

element: &'a [ast::PathSegment],

elements: &'a [ast::PathSegment],

or segments or path?

fmease · 2026-05-22T14:09:43Z

+                .filter_map(|segment| {
+                    if !segment.ident.is_special() { Some(segment.ident.as_str()) } else { None }
+                })


Fair. However if you were to map kw::PathRoot to "" here, you wouldn't need global_path, right?

scrabsha · 2026-05-23T07:53:42Z

hi @fmease, thank you infinitely for your review. you put a lot of care into these and i really really appreciate all your explanations <3

i'll fix everything once the all hands is over and i have some quiet time, but I still wanted to say thanks as soon as possible :)

thank you again!

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 13, 2026

This comment has been minimized.

Sign in to view

scrabsha force-pushed the push-qlmmuuwqnoqv branch from 291ff75 to 290c04c Compare April 13, 2026 19:54

fmease reviewed Apr 13, 2026

View reviewed changes

Comment thread compiler/rustc_parse/src/parser/item.rs Outdated

scrabsha force-pushed the push-qlmmuuwqnoqv branch from 290c04c to 83f6ae9 Compare April 14, 2026 20:11

fmease self-assigned this Apr 14, 2026

scrabsha added 2 commits April 15, 2026 20:41

Add test for attribute in use tree

d08c828

Recover on attribute in use tree

0e53906

scrabsha force-pushed the push-qlmmuuwqnoqv branch from 83f6ae9 to 0e53906 Compare April 15, 2026 21:07

scrabsha commented Apr 17, 2026

View reviewed changes

scrabsha marked this pull request as ready for review April 17, 2026 15:53

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 Apr 17, 2026

fmease reviewed May 22, 2026

View reviewed changes

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 22, 2026

		if p.check_noexpect(&TokenKind::Pound)
		&& p.look_ahead(1, \|tok\| matches!(tok.kind, TokenKind::OpenBracket))

	.span_to_source(use_tree_span, \|s, start, end\| Ok(s[start..end].to_string()))
	.span_to_snippet(use_tree_span)

	.span_to_source(attr_span, \|s, start, end\| Ok(s[start..end].to_string()))
	.span_to_snippet(attr_span)

	element: &'a [ast::PathSegment],
	elements: &'a [ast::PathSegment],

Uh oh!

Uh oh!

Conversation

scrabsha commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

scrabsha commented Apr 14, 2026

Uh oh!

fmease commented Apr 14, 2026

Uh oh!

scrabsha Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026

Choose a reason for hiding this comment

Uh oh!

scrabsha Apr 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scrabsha commented Apr 17, 2026

Uh oh!

rustbot commented Apr 17, 2026

Uh oh!

rustbot commented Apr 17, 2026

Uh oh!

fmease left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 3, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 22, 2026

Choose a reason for hiding this comment

Uh oh!

scrabsha commented May 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

scrabsha commented Apr 13, 2026 •

edited

Loading

scrabsha Apr 17, 2026 •

edited

Loading

scrabsha Apr 17, 2026 •

edited by rustbot

Loading

fmease left a comment •

edited by rustbot

Loading

fmease May 3, 2026 •

edited

Loading

fmease May 22, 2026 •

edited

Loading

fmease May 22, 2026 •

edited by rustbot

Loading

fmease left a comment •

edited by rustbot

Loading

fmease May 22, 2026 •

edited

Loading