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

Handle C-variadic arguments properly when reporting region errors #86164

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

FabianWolff
Copy link
Contributor

This pull request fixes #86053. The issue is that for a C-variadic function

#![feature(c_variadic)]
unsafe extern "C" fn foo(_: (), ...) {}

foo's signature will contain only the first parameter (and have c_variadic set to true), whereas its body has a second argument (a hir::Pat for the ...).

The code for reporting region errors iterates over the body's parameters and tries to fetch the corresponding parameter from the signature; this causes an out-of-bounds ICE for the ... (though not in the example above, because there are no region errors to report).

I have simply restricted the iteration over the body parameters to exclude ..., which is fine because ... cannot cause a region error.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 9, 2021
@rust-log-analyzer

This comment has been minimized.


fn ordering4 < 'a , 'b > ( a : , self , self , self ,
self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
}
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be formatted like this or can you run rustfmt on it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't need to be, but since this is a regression test, I have tried to reproduce the example from #86053 as closely as possible (the line break had to happen in order to make test tidy happy).

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

Apologies for the delay in reviewing, this looks good to me.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2021

📌 Commit 7dccce0 has been approved by davidtwco

@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 Jun 16, 2021
@bors
Copy link
Contributor

bors commented Jun 17, 2021

⌛ Testing commit 7dccce0 with merge cb3c4ee...

@bors
Copy link
Contributor

bors commented Jun 17, 2021

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing cb3c4ee to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 17, 2021
@bors bors merged commit cb3c4ee into rust-lang:master Jun 17, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
7 participants