Skip to content

Conversation

@cyrgani
Copy link
Contributor

@cyrgani cyrgani commented Oct 18, 2025

Before this PR, calling TokenStream::from_str or Literal::from_str with an invalid argument would always cause a compile error, even if the TokenStream is not used afterwards at all.
This PR changes this so it returns a LexError instead in some cases.

This is very theoretically a breaking change, but the doc comment on the impl already says

/// NOTE: some errors may cause panics instead of returning `LexError`. We reserve the right to
/// change these errors into `LexError`s later.

Fixes some cases of #58736.

@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Oct 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
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

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@cyrgani
Copy link
Contributor Author

cyrgani commented Oct 18, 2025

@rustbot author

@rustbot rustbot 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 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@cyrgani cyrgani force-pushed the nonfatal-tokenstream-parse branch from e2c0009 to 80b67bb Compare October 19, 2025 09:03
@cyrgani
Copy link
Contributor Author

cyrgani commented Oct 19, 2025

@rustbot ready

@rustbot 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 Oct 19, 2025
@cyrgani cyrgani force-pushed the nonfatal-tokenstream-parse branch from ba098bc to 0274d78 Compare October 21, 2025 14:28
@cyrgani
Copy link
Contributor Author

cyrgani commented Oct 23, 2025

This does not fix #58736 entirely yet:

TokenStream::from_str("0b2");

is still a compile error because it internally calls literal_from_str, which then emits the diagnostic.

@davidtwco
Copy link
Member

I'm not familiar enough with this part of the compiler

r? compiler

@rustbot rustbot assigned wesleywiser and unassigned davidtwco Oct 25, 2025
@cyrgani cyrgani force-pushed the nonfatal-tokenstream-parse branch from 0274d78 to 13d3d06 Compare October 29, 2025 22:19
@rustbot

This comment has been minimized.

@cyrgani cyrgani changed the title replace TokenStream::from_str panics with LexErrors reduce the amount of panics in {TokenStream, Literal}::from_str calls Oct 29, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
JaclynCodes added a commit to JaclynCodes/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
rust-timer added a commit that referenced this pull request Nov 17, 2025
Rollup merge of #148505 - cyrgani:pm-tests, r=madsmtm

add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as #147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Nov 17, 2025
add larger test for `proc_macro` `FromStr` implementations

Currently, there are only few tests that check the output of `TokenStream::from_str` and `Literal::from_str` (which is somewhat understandable as the rustc implementation just delegates these calls to the parser). In preparation for both the standalone backend (rust-lang/rust#130856) which will probably need to reimplement this logic as well as for removing panics from these functions (rust-lang/rust#58736), this PR adds a test which shows the various messy ways of how these functions report errors and the return values for successful parses.
Followup PRs such as rust-lang/rust#147859 will change more and more of these "diagnostic + error"s into `LexErrors`.

The test structure with the extra module is used to allow reusing it later easily for the standalone backend.
@cyrgani cyrgani force-pushed the nonfatal-tokenstream-parse branch from 13d3d06 to faf91ab Compare November 17, 2025 15:27
@rustbot
Copy link
Collaborator

rustbot commented Nov 17, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants