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

Don't resolve blocks in foreign functions #92510

Merged
merged 2 commits into from
Jan 9, 2022

Conversation

inquisitivecrystal
Copy link
Contributor

Although it is an error for a foreign function to have a block, it is still possible at the level of the AST. #74204 made AST lowering skip over blocks belonging to foreign functions, since they're invalid. However, resolve still treated these blocks normally, resulting in a mismatch between the HIR and resolve, which could cause an ICE under certain circumstances. This PR changes resolve to skip over blocks belonging to foreign functions, as AST lowering does.

Fixes #91370.

r? @cjgillot

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 3, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 3, 2022
@inquisitivecrystal inquisitivecrystal added the A-resolve Area: Name resolution label Jan 3, 2022
@inquisitivecrystal inquisitivecrystal changed the title Do not resolve blocks in foreign functions Don't resolve blocks in foreign functions Jan 3, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Jan 3, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jan 3, 2022

📌 Commit f56a6527a546b1a9555fb4ae7f8dd90d23a35483 has been approved by cjgillot

@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 Jan 3, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Jan 3, 2022

@bors r- too fast

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 3, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Jan 3, 2022

r=me with nit addressed. Thanks @inquisitivecrystal!

@inquisitivecrystal inquisitivecrystal marked this pull request as ready for review January 5, 2022 23:26
@inquisitivecrystal
Copy link
Contributor Author

@cjgillot Thank you for your advice! The nits have been addressed. Unfortunately, I don't have r+ privileges. Could you merge?

@inquisitivecrystal inquisitivecrystal 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 Jan 6, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Jan 6, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jan 6, 2022

📌 Commit 42de973 has been approved by cjgillot

@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 Jan 6, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 6, 2022
…=cjgillot

Don't resolve blocks in foreign functions

Although it is an error for a foreign function to have a block, it is still possible at the level of the AST. rust-lang#74204 made AST lowering skip over blocks belonging to foreign functions, since they're invalid. However, resolve still treated these blocks normally, resulting in a mismatch between the HIR and resolve, which could cause an ICE under certain circumstances. This PR changes resolve to skip over blocks belonging to foreign functions, as AST lowering does.

Fixes rust-lang#91370.

r? `@cjgillot`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 9, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#92055 (Add release notes for 1.58)
 - rust-lang#92490 (Move crate drop-down to search results page)
 - rust-lang#92510 (Don't resolve blocks in foreign functions)
 - rust-lang#92573 (expand: Refactor InvocationCollector visitor for better code reuse)
 - rust-lang#92608 (rustdoc: Introduce a resolver cache for sharing data between early doc link resolution and later passes)
 - rust-lang#92657 (Implemented const casts of raw pointers)
 - rust-lang#92671 (Make `Atomic*::from_mut` return `&mut Atomic*`)
 - rust-lang#92673 (Remove useless collapse toggle on "all items" page)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 464a081 into rust-lang:master Jan 9, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution 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 when a function inside an extern block has a body, and that body has a trait impl
5 participants