Don't visit foreign function bodies when lowering ast to hir

Previously the existence of bodies inside a foreign function block would
cause a panic in the hir `NodeCollector` during its collection of crate
bodies to compute a crate hash:

e59b08e62e/src/librustc_middle/hir/map/collector.rs (L154-L158)

The collector walks the hir tree and creates a map of hir nodes, then
attaching bodies in the crate to their owner in the map. For a code like

```rust
extern "C" {
    fn f() {
        fn g() {}
    }
}
```

The crate bodies include the body of the function `g`. But foreign
functions cannot have bodies, and while the parser AST permits a foreign
function to have a body, the hir doesn't. This means that the body of
`f` is not present in the hir, and so neither is `g`. So when the
`NodeCollector` finishes the walking the hir, it has no record of `g`,
cannot find an owner for the body of `g` it sees in the crate bodies,
and blows up.

Why do the crate bodies include the body of `g`? The AST walker has a
need a for walking function bodies, and FFIs share the same AST node as
functions in other contexts.

There are at least two options to fix this:

- Don't unwrap the map entry for an hir node in the `NodeCollector`
- Modifier the ast->hir lowering visitor to ignore foreign function
  blocks

I don't think the first is preferrable, since we want to know when we
can't find a body for an hir node that we thought had one (dropping this
information may lead to an invalid hash). So this commit implements the
second option.

Closes #74120
This commit is contained in:
Ayaz Hafiz 2020-07-09 19:03:15 -07:00
parent 8ac1525e09
commit 230393993f
No known key found for this signature in database
GPG Key ID: B443F7A3030C9AED
3 changed files with 69 additions and 1 deletions

View File

@ -6,7 +6,8 @@ use rustc_ast::ast::*;
use rustc_ast::attr;
use rustc_ast::node_id::NodeMap;
use rustc_ast::ptr::P;
use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor};
use rustc_ast::walk_list;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
@ -76,6 +77,43 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
}
}
// Forked from the original method because we don't want to descend into foreign function
// blocks. Such blocks are semantically invalid and the hir does not preserve them, so lowering
// items contained in them may be unexpected by later passes.
fn visit_foreign_item(&mut self, item: &'a ForeignItem) {
let Item { id: _, span: _, ident, ref vis, ref attrs, ref kind, tokens: _ } = *item;
self.visit_vis(vis);
self.visit_ident(ident);
walk_list!(self, visit_attribute, attrs);
match kind {
ForeignItemKind::Static(ty, _, expr) => {
self.visit_ty(ty);
walk_list!(self, visit_expr, expr);
}
ForeignItemKind::Fn(_, sig, generics, body) => {
self.visit_generics(generics);
let kind = FnKind::Fn(FnCtxt::Foreign, ident, sig, vis, body.as_deref());
match kind {
FnKind::Fn(_, _, sig, _, _) => {
self.visit_fn_header(&sig.header);
visit::walk_fn_decl(self, &sig.decl);
}
FnKind::Closure(decl, _) => {
visit::walk_fn_decl(self, decl);
}
}
}
ForeignItemKind::TyAlias(_, generics, bounds, ty) => {
self.visit_generics(generics);
walk_list!(self, visit_param_bound, bounds);
walk_list!(self, visit_ty, ty);
}
ForeignItemKind::MacCall(mac) => {
self.visit_mac(mac);
}
}
}
fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
self.lctx.with_hir_id_owner(item.id, |lctx| match ctxt {
AssocCtxt::Trait => {

View File

@ -0,0 +1,11 @@
// Previously this ICE'd because `fn g()` would be lowered, but the block associated with `fn f()`
// wasn't.
// compile-flags: --crate-type=lib
extern "C" {
fn f() {
//~^ incorrect function inside `extern` block
fn g() {}
}
}

View File

@ -0,0 +1,19 @@
error: incorrect function inside `extern` block
--> $DIR/issue-74120-lowering-of-ffi-block-bodies.rs:7:8
|
LL | extern "C" {
| ---------- `extern` blocks define existing foreign functions and functions inside of them cannot have a body
LL | fn f() {
| ________^___-
| | |
| | cannot have a body
LL | |
LL | | fn g() {}
LL | | }
| |_____- help: remove the invalid body: `;`
|
= help: you might have meant to write a function accessible through FFI, which can be done by writing `extern fn` outside of the `extern` block
= note: for more information, visit https://doc.rust-lang.org/std/keyword.extern.html
error: aborting due to previous error