Skip to content

Avoid crashing on variadic functions when producing arg-mismatch errors #130437

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
Merged
merged 1 commit into from
Sep 16, 2024
Merged

Avoid crashing on variadic functions when producing arg-mismatch errors #130437

merged 1 commit into from
Sep 16, 2024

Conversation

jder
Copy link
Contributor

@jder jder commented Sep 16, 2024

Fixes #130372 by accommodating how variadic functions change the argument list length between HIR body and FnDecls.

Also degrades the zip_eq to a debug_assert! to match other asserts in the area to avoid being disruptive to users. There is at least one other crash in this area I am working on in #130400 and also considering how we might refactor some of this code to hoist some of this logic up higher.

r? @compiler-errors

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

compiler-errors commented Sep 16, 2024 •
edited
Loading

I think you may need to remove some crashes tests. Try running ./x.py test crashes --bless, after you make sure you've rebased onto master (git pull [email protected]:rust-lang/rust.git master --rebase).

@jder
Copy link
Contributor Author

jder commented Sep 16, 2024

Thanks, I will look. I also realized I should verify there isn't a separate issue here with delegation before we close the linked issue.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@matthiaskrgr
Copy link
Member

I don't think --bless has any influence on the crashes tests, I never implemened anything like that at least 🤔

@compiler-errors
Copy link
Member

compiler-errors commented Sep 16, 2024 •
edited
Loading

Oh, yeah, the bless was just a reflex. The point was to test them.

(Off-topic, but for the record, I would prefer if --bless didn't automatically remove the tests since I would expect users to have to manually think about what to do with a crashes test.)

@jder
Copy link
Contributor Author

jder commented Sep 16, 2024

Thanks, I got rid of the now-not-crashing tests, all of which are the same issue as the new test I added. (Sorry for the noise, I swear next time I will remember that crashes are usually already checked in by the time I go to fix them.)

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 16, 2024

📌 Commit 45eceb2 has been approved by compiler-errors

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 Sep 16, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 16, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 16, 2024
@bors
Auto merge of rust-lang#130444 - matthiaskrgr:rollup-onlrjva, r=matth…
c52c23b 
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#130033 (Don't call `fn_arg_names` query for non-`fn` foreign items in resolver)
 - rust-lang#130282 (Do not report an excessive number of overflow errors for an ever-growing deref impl)
 - rust-lang#130437 (Avoid crashing on variadic functions when producing arg-mismatch errors)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 14ee69c into rust-lang:master Sep 16, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 16, 2024
@rust-timer
Unrolled build for rust-lang#130437
500919d 
Rollup merge of rust-lang#130437 - jder:issue-130372, r=compiler-errors

Avoid crashing on variadic functions when producing arg-mismatch errors

Fixes rust-lang#130372 by accommodating how variadic functions change the argument list length between HIR body and FnDecls.

Also degrades the zip_eq to a debug_assert! to match other asserts in the area to avoid being disruptive to users. There is at least one other crash in this area I am working on in rust-lang#130400 and also considering how we might refactor some of this code to hoist some of this logic up higher.

r? `@compiler-errors`
@rustbot rustbot added this to the 1.83.0 milestone Sep 16, 2024
@jder jder deleted the issue-130372 branch September 17, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

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
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.

ICE: itertools: .zip_eq() reached end of one iterator before the other
6 participants
@jder @compiler-errors @rust-log-analyzer @matthiaskrgr @bors @rustbot