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.

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

Detect invalid exprs in parser used by pretty-printer tests #134599

Merged
merged 4 commits into from
Dec 22, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Dec 21, 2024

This PR fixes a bug in #133730 inherited from #43742. Before this fix, the test might silently only operate on a prefix of some of the test cases in this table:

static EXPRS: &[&str] = &[

For example, adding the test case 1 .. 2 .. 3 (a syntactically invalid expression) into the table would unexpectedly succeed the test instead of crashing at this unwrap:

for &source_code in EXPRS {
let expr = parse_expr(psess, source_code).unwrap();

because parse_expr would successfully parse just 1 .. 2 and disregard the last .. 3.

This PR adds a check that parse_expr reaches Eof, ensuring all the test cases actually test the whole expression they look like they are supposed to.

@dtolnay dtolnay added the A-pretty Area: Pretty printing (including `-Z unpretty`) label Dec 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 21, 2024

r? @Mark-Simulacrum

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

@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. labels Dec 21, 2024
Copy link
Member

Choose a reason for hiding this comment

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

In both pprust-*.rs tests, I think I would rather use //@ aux-crate: parser=parser.rs //@ edition: 2021 (aux-crate puts the crate into the extern prelude contrary to aux-build) instead of fighting against compiletest (#[path = "auxiliary/parser.rs"] mod parser;). The extra overhead is not really relevant.

@fmease
Copy link
Member

fmease commented Dec 22, 2024

r? fmease @bors rollup

r=me with nit addressed in one way or another and a bit of commit squashing

@rustbot rustbot assigned fmease and unassigned Mark-Simulacrum Dec 22, 2024
@dtolnay
Copy link
Member Author

dtolnay commented Dec 22, 2024

@bors r=fmease

@bors
Copy link
Contributor

bors commented Dec 22, 2024

📌 Commit 1f2028f has been approved by fmease

It is now in the queue for this repository.

@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 Dec 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2024
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#134599 (Detect invalid exprs in parser used by pretty-printer tests)
 - rust-lang#134602 (Document `PointerLike` implementation restrictions.)
 - rust-lang#134635 (Don't ICE on illegal `dyn*` casts)
 - rust-lang#134639 (Make sure we note ambiguity causes on positive/negative impl conflicts)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 87be70e into rust-lang:master Dec 22, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 22, 2024
Rollup merge of rust-lang#134599 - dtolnay:fulldepsparser, r=fmease

Detect invalid exprs in parser used by pretty-printer tests

This PR fixes a bug in rust-lang#133730 inherited from rust-lang#43742. Before this fix, the test might silently only operate on a prefix of some of the test cases in this table:

https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/tests/ui-fulldeps/pprust-parenthesis-insertion.rs#L57

For example, adding the test case `1 .. 2 .. 3` (a syntactically invalid expression) into the table would unexpectedly succeed the test instead of crashing at this unwrap:

https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/tests/ui-fulldeps/pprust-parenthesis-insertion.rs#L199-L200

because `parse_expr` would successfully parse just `1 .. 2` and disregard the last `.. 3`.

This PR adds a check that `parse_expr` reaches `Eof`, ensuring all the test cases actually test the whole expression they look like they are supposed to.
@dtolnay dtolnay deleted the fulldepsparser branch December 22, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pretty Area: Pretty printing (including `-Z unpretty`) 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
Development

Successfully merging this pull request may close these issues.

5 participants