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

Fix invalid silencing of parsing error #123223

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Conversation

estebank
Copy link
Contributor

Given

macro_rules! a {
    ( ) => {
        impl<'b> c for d {
            e::<f'g>
        }
    };
}

ensure an error is emitted.

Fix #123079.

@rustbot
Copy link
Collaborator

rustbot commented Mar 30, 2024

r? @pnkfelix

rustbot has assigned @pnkfelix.
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 Mar 30, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

So it turns out that this still ICEs:

macro_rules! a {
    ( ) => {
        impl<'b> c for f'f {}
    };
}
fn main() {}

Specifically, this ICE example above suggests that having an exhaustive list of tokens to disable this behavior (self.last_lifetime = None;) is fragile.

I think the right solution is to not delay a bug at all for this class of errors. If the redundancy is too noisy, we should instead be using diagnostic stashing to actually ensure that at least one error is emitted.

@compiler-errors
Copy link
Member

@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 Mar 30, 2024
@compiler-errors
Copy link
Member

Beginning to think that the right solution is just to revert ea1883d -- suppressing the prefix error seems nice in theory, but perhaps not worth the complication of getting it right. Curious what @fmease thinks, since he suggested this.

@fmease
Copy link
Member

fmease commented Mar 30, 2024

fine by me 👍 totally missed that this was so fragile during my review, we probably shouldn't keep patching this like that

@estebank
Copy link
Contributor Author

@compiler-errors yeah, I agree. I spend some time last night trying to come up with counter examples or alternative approcaches, but it got late enough that I convinced myself that removing the delay was likely the best thing to do.

@estebank estebank 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 Mar 30, 2024
@rust-log-analyzer

This comment has been minimized.

Given

```rust
macro_rules! a {
    ( ) => {
        impl<'b> c for d {
            e::<f'g>
        }
    };
}
```

ensure an error is emitted.

Fix rust-lang#123079.
@pnkfelix
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 12, 2024

📌 Commit e572a19 has been approved by pnkfelix

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

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123204 (rustdoc: point at span in `include_str!`-ed md file)
 - rust-lang#123223 (Fix invalid silencing of parsing error)
 - rust-lang#123249 (do not add prolog for variadic naked functions)
 - rust-lang#123825 (Call the panic hook for non-unwind panics in proc-macros)
 - rust-lang#123833 (Update stdarch submodule)
 - rust-lang#123841 (Improve diagnostic by suggesting to remove visibility qualifier)
 - rust-lang#123849 (Update E0384.md)
 - rust-lang#123852 (fix typo in library/std/src/lib.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 68359e2 into rust-lang:master Apr 12, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 12, 2024
Rollup merge of rust-lang#123223 - estebank:issue-123079, r=pnkfelix

Fix invalid silencing of parsing error

Given

```rust
macro_rules! a {
    ( ) => {
        impl<'b> c for d {
            e::<f'g>
        }
    };
}
```

ensure an error is emitted.

Fix rust-lang#123079.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

ICE: prefix is unknown
7 participants