Strip frontmatter in fewer places by fmease · Pull Request #146340 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation18 Commits2 Checks10 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes #145945.
Fixes #146132.

r? fee1-dead

rustbot added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

T-clippy

Relevant to the Clippy team.

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.

T-rustfmt

Relevant to the rustfmt team, which will review and decide on the PR/issue.

labels

Sep 8, 2025

This comment has been minimized.

self.psess(),
name,
s.to_owned(),
StripTokens::Nothing,

Choose a reason for hiding this comment

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

Updating this to not strip shebangs isn't breaking.

It already compares spans below to ensure no extra trivia is contained like whitespace, comments or – well – shebangs. However, it's just cleaner to pass Nothing here.

///
/// On failure, the errors must be consumed via `unwrap_or_emit_fatal`, `emit`, `cancel`,
/// etc., otherwise a panic will occur when they are dropped.
pub fn new_parser_from_simple_source_str(

Choose a reason for hiding this comment

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

Removing this convenience wrapper (for StripTokens::Nothing) to force all callers to have to think what behavior they want which is a lot more robust. Otherwise, people will just overlook this function and blindly go for the "normal" variant new_parser_from_source_str.

psess: &ParseSess,
name: FileName,
source: String,
override_span: Option<Span>,

Choose a reason for hiding this comment

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

I might add strip_tokens: StripTokens as a parameter to this function in a future PR. At the time of writing, it wouldn't really matter but there are some call sites like the one for -Zcrate-attr where we could pass StripTokens::Nothing (since the input is wrapped in #![ ] anyway) but it wouldn't lead to any observable difference in rustc's behavior.

&psess,
file_name,
snippet.clone(),
StripTokens::Nothing,

Choose a reason for hiding this comment

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

Shouldn't make a difference since this is for (re)parsing macro matchers which will have already been validated by rustc and shebangs or frontmatters aren't part of that subgrammar.

Choose a reason for hiding this comment

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

This is just test code; use StripTokens::Nothing for simplicity. If anybody wants to test shebangs and/or frontmatter in here, they can just update it.

};
// Don't strip any tokens; it wouldn't matter anyway because the source is wrapped in a function.
let mut parser =
match new_parser_from_source_str(&psess, filename, wrapped_source, StripTokens::Nothing) {

Choose a reason for hiding this comment

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

Change is unobservable as the added comment explains.

let parser = rustc_parse::unwrap_or_emit_fatal(rustc_parse::new_parser_from_source_str(
psess,
FileName::anon_source_code(source_code),
rustc_parse::lexer::StripTokens::Nothing,

Choose a reason for hiding this comment

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

This is also just test code.

fmease marked this pull request as ready for review

September 9, 2025 16:22

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

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

Sep 9, 2025

@Urgau Urgau left a comment • Loading

Choose a reason for hiding this comment

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

This comment has been minimized.

Choose a reason for hiding this comment

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

@bors r=fee1-dead,Urgau rollup

📌 Commit 7a66925 has been approved by fee1-dead,Urgau

It is now in the queue for this repository.

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

Sep 9, 2025

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Sep 9, 2025

…fee1-dead,Urgau

Strip frontmatter in fewer places

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes rust-lang#145945. Fixes rust-lang#146132.

r? fee1-dead

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Sep 9, 2025

…fee1-dead,Urgau

Strip frontmatter in fewer places

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes rust-lang#145945. Fixes rust-lang#146132.

r? fee1-dead

bors added a commit that referenced this pull request

Sep 10, 2025

Rollup of 6 pull requests

Successful merges:

#146311 (Minor symbol comment fixes.)
#146340 (Strip frontmatter in fewer places)
#146342 (Improve C-variadic error messages: part 2)
#146347 (report duplicate symbols added by the driver)
#146374 (Update browser-ui-test version to 0.22.2)
#146379 (Fix compare_against_sw_vers test)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Sep 10, 2025

Rollup of 8 pull requests

Successful merges:

#145327 (std: make address resolution weirdness local to SGX)
#145879 (default auto traits: use default supertraits instead of Self: Trait bounds on associated items)
#146123 (Suggest examples of format specifiers in error messages)
#146311 (Minor symbol comment fixes.)
#146322 (Make Barrier RefUnwindSafe again)
#146327 (Add tests for deref on pin)
#146340 (Strip frontmatter in fewer places)
#146342 (Improve C-variadic error messages: part 2)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit that referenced this pull request

Sep 11, 2025

Rollup merge of #146340 - fmease:frontmatter-containment, r=fee1-dead,Urgau

Strip frontmatter in fewer places

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes #145945. Fixes #146132.

r? fee1-dead

fmease deleted the frontmatter-containment branch

September 11, 2025 00:44

epage mentioned this pull request

Sep 16, 2025

11 tasks

flip1995 pushed a commit to flip1995/rust that referenced this pull request

Sep 18, 2025

…fee1-dead,Urgau

Strip frontmatter in fewer places

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes rust-lang#145945. Fixes rust-lang#146132.

r? fee1-dead

Muscraft pushed a commit to Muscraft/rust that referenced this pull request

Sep 24, 2025

…fee1-dead,Urgau

Strip frontmatter in fewer places

Stop stripping frontmatter in proc_macro::Literal::from_str (RUST-146132)
Stop stripping frontmatter in expr-ctxt (but not item-ctxt!) includes (RUST-145945)
Stop stripping shebang (!) in proc_macro::Literal::from_str
- Not a breaking change because it did compare spans already to ensure there wasn't extra whitespace or comments (Literal::from_str("#!\n0") already yields Err(_) thankfully!)
Stop stripping frontmatter+shebang inside some rustdoc code where it doesn't make any observable difference (see self review comments)
(Stop stripping frontmatter+shebang inside internal test code)

Fixes rust-lang#145945. Fixes rust-lang#146132.

r? fee1-dead

epage mentioned this pull request

Oct 23, 2025

10 tasks

Labels

F-frontmatter

`#![feature(frontmatter)]`

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-clippy

Relevant to the Clippy team.

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.

T-rustfmt

Relevant to the rustfmt team, which will review and decide on the PR/issue.