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.

Sign up for GitHub

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

Jump to bottom

Always force non-trimming of path in unreachable_patterns lint #135310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Always force non-trimming of path in unreachable_patterns lint #135310

wants to merge 1 commit into from
+1 −8

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 9, 2025

Creating a "trimmed DefID path" when no error is being emitted is an ICE (on purpose). If we create a trimmed path for a lint that is then silenced before being emitted causes a known ICE. This side-steps the issue by always using with_no_trimmed_path!.

This was verified to fix https://github.com/quinn-rs/quinn/, but couldn't write a repro case for the test suite.

Fix #135289.

@estebank
Always force non-trimming of path in unreachable_patterns lint
93a1950 
Creating a "trimmed DefID path" when no error is being emitted is an ICE (on purpose). If we create a trimmed path for a lint that is then silenced before being emitted causes a known ICE. This side-steps the issue by always using `with_no_trimmed_path!`.

This was verified to fix https://github.com/quinn-rs/quinn/, but couldn't write a repro case for the test suite.

Fix rust-lang#135289.
@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2025

r? @Nadrieril

rustbot has assigned @Nadrieril.
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 Jan 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2025

Some changes occurred in match checking

cc @Nadrieril

@estebank estebank mentioned this pull request Jan 9, 2025
Open
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
curl: (22) The requested URL returned error: 404
curl: (22) The requested URL returned error: 404
ERROR: failed to download llvm from ci

    HELP: There could be two reasons behind this:
        1) The host triple is not supported for `download-ci-llvm`.
        2) Old builds get deleted after a certain time.
    HELP: In either case, disable `download-ci-llvm` in your config.toml:
    [llvm]
    download-ci-llvm = false
    
Build completed unsuccessfully in 0:00:22

@lqd
Copy link
Member

lqd commented Jan 9, 2025

I had the same workaround locally, but still wondered where the lint was silenced for the unreachable arms in that crate 🤔.

@estebank
Copy link
Contributor Author

estebank commented Jan 9, 2025

@lqd my assumption it is because of cargo silencing the lint for deps, but it's only a guess.

@lqd
Copy link
Member

lqd commented Jan 10, 2025 •
edited
Loading

Interesting possibility. They do that via --cap-lints=allow IIRC, so I would have expected this to allow unreachable patterns, that you check at the beginning of the typo const checking. But maybe they do so differently for this use-case via tests.

@compiler-errors
Copy link
Member

Would be nice to have a test for this

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

@Nadrieril Nadrieril

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ICE: trimmed_def_paths called, diagnostics were expected but none were emitted
6 participants
@estebank @rustbot @rust-log-analyzer @lqd @compiler-errors @Nadrieril