Skip to content

Commit

Permalink
Rollup merge of #64250 - Xanewok:save-analysis-assoc-nested, r=varkor
Browse files Browse the repository at this point in the history
save-analysis: Nest typeck tables when processing functions/methods

Fixes an issue where we did not nest tables correctly when resolving
associated types in formal argument/return type positions.

This was the minimized reproduction case that I tested the fix on:
```rust
pub trait Trait {
    type Assoc;
}

pub struct A;

pub fn func() {
    fn _inner1<U: Trait>(_: U::Assoc) {}
    fn _inner2<U: Trait>() -> U::Assoc { unimplemented!() }

    impl A {
        fn _inner1<U: Trait>(self, _: U::Assoc) {}
        fn _inner2<U: Trait>(self) -> U::Assoc { unimplemented!() }
    }
}
```
using `debug_assertions`-enabled rustc and by additionally passing `-Zsave-analysis`.

Unfortunately the original assertion fired is a *debug* one and from what I can tell we don't run the tests with these on, so I'm not adding a test here. If I missed it and there is a way to run tests with these on, I'd love to add a test case for this.

Closes #63663
Closes #50328
Closes #43982
  • Loading branch information
tmandry authored Sep 10, 2019
2 parents 71f3fbc + 7730ef1 commit 656ce3f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 62 deletions.
38 changes: 18 additions & 20 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,26 +205,24 @@ pub struct LocalTableInContext<'a, V> {
fn validate_hir_id_for_typeck_tables(local_id_root: Option<DefId>,
hir_id: hir::HirId,
mut_access: bool) {
if cfg!(debug_assertions) {
if let Some(local_id_root) = local_id_root {
if hir_id.owner != local_id_root.index {
ty::tls::with(|tcx| {
bug!("node {} with HirId::owner {:?} cannot be placed in \
TypeckTables with local_id_root {:?}",
tcx.hir().node_to_string(hir_id),
DefId::local(hir_id.owner),
local_id_root)
});
}
} else {
// We use "Null Object" TypeckTables in some of the analysis passes.
// These are just expected to be empty and their `local_id_root` is
// `None`. Therefore we cannot verify whether a given `HirId` would
// be a valid key for the given table. Instead we make sure that
// nobody tries to write to such a Null Object table.
if mut_access {
bug!("access to invalid TypeckTables")
}
if let Some(local_id_root) = local_id_root {
if hir_id.owner != local_id_root.index {
ty::tls::with(|tcx| {
bug!("node {} with HirId::owner {:?} cannot be placed in \
TypeckTables with local_id_root {:?}",
tcx.hir().node_to_string(hir_id),
DefId::local(hir_id.owner),
local_id_root)
});
}
} else {
// We use "Null Object" TypeckTables in some of the analysis passes.
// These are just expected to be empty and their `local_id_root` is
// `None`. Therefore we cannot verify whether a given `HirId` would
// be a valid key for the given table. Instead we make sure that
// nobody tries to write to such a Null Object table.
if mut_access {
bug!("access to invalid TypeckTables")
}
}
}
Expand Down
80 changes: 38 additions & 42 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,36 +283,32 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {
) {
debug!("process_method: {}:{}", id, ident);

if let Some(mut method_data) = self.save_ctxt.get_method_data(id, ident, span) {
let sig_str = crate::make_signature(&sig.decl, &generics);
if body.is_some() {
self.nest_tables(
id,
|v| v.process_formals(&sig.decl.inputs, &method_data.qualname),
);
}
let hir_id = self.tcx.hir().node_to_hir_id(id);
self.nest_tables(id, |v| {
if let Some(mut method_data) = v.save_ctxt.get_method_data(id, ident, span) {
v.process_formals(&sig.decl.inputs, &method_data.qualname);
v.process_generic_params(&generics, &method_data.qualname, id);

self.process_generic_params(&generics, &method_data.qualname, id);
method_data.value = crate::make_signature(&sig.decl, &generics);
method_data.sig = sig::method_signature(id, ident, generics, sig, &v.save_ctxt);

method_data.value = sig_str;
method_data.sig = sig::method_signature(id, ident, generics, sig, &self.save_ctxt);
let hir_id = self.tcx.hir().node_to_hir_id(id);
self.dumper.dump_def(&access_from_vis!(self.save_ctxt, vis, hir_id), method_data);
}
v.dumper.dump_def(&access_from_vis!(v.save_ctxt, vis, hir_id), method_data);
}

// walk arg and return types
for arg in &sig.decl.inputs {
self.visit_ty(&arg.ty);
}
// walk arg and return types
for arg in &sig.decl.inputs {
v.visit_ty(&arg.ty);
}

if let ast::FunctionRetTy::Ty(ref ret_ty) = sig.decl.output {
self.visit_ty(ret_ty);
}
if let ast::FunctionRetTy::Ty(ref ret_ty) = sig.decl.output {
v.visit_ty(ret_ty);
}

// walk the fn body
if let Some(body) = body {
self.nest_tables(id, |v| v.visit_block(body));
}
// walk the fn body
if let Some(body) = body {
v.visit_block(body);
}
});
}

fn process_struct_field_def(&mut self, field: &ast::StructField, parent_id: NodeId) {
Expand Down Expand Up @@ -377,26 +373,26 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {
ty_params: &'l ast::Generics,
body: &'l ast::Block,
) {
if let Some(fn_data) = self.save_ctxt.get_item_data(item) {
down_cast_data!(fn_data, DefData, item.span);
self.nest_tables(
item.id,
|v| v.process_formals(&decl.inputs, &fn_data.qualname),
);
self.process_generic_params(ty_params, &fn_data.qualname, item.id);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.dumper.dump_def(&access_from!(self.save_ctxt, item, hir_id), fn_data);
}
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.nest_tables(item.id, |v| {
if let Some(fn_data) = v.save_ctxt.get_item_data(item) {
down_cast_data!(fn_data, DefData, item.span);
v.process_formals(&decl.inputs, &fn_data.qualname);
v.process_generic_params(ty_params, &fn_data.qualname, item.id);

for arg in &decl.inputs {
self.visit_ty(&arg.ty);
}
v.dumper.dump_def(&access_from!(v.save_ctxt, item, hir_id), fn_data);
}

if let ast::FunctionRetTy::Ty(ref ret_ty) = decl.output {
self.visit_ty(&ret_ty);
}
for arg in &decl.inputs {
v.visit_ty(&arg.ty)
}

self.nest_tables(item.id, |v| v.visit_block(&body));
if let ast::FunctionRetTy::Ty(ref ret_ty) = decl.output {
v.visit_ty(&ret_ty);
}

v.visit_block(&body);
});
}

fn process_static_or_const_item(
Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/save-analysis/issue-63663.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// check-pass
// compile-flags: -Zsave-analysis

// Check that this doesn't ICE when processing associated const in formal
// argument and return type of functions defined inside function/method scope.

pub trait Trait {
type Assoc;
}

pub struct A;

pub fn func() {
fn _inner1<U: Trait>(_: U::Assoc) {}
fn _inner2<U: Trait>() -> U::Assoc { unimplemented!() }

impl A {
fn _inner1<U: Trait>(self, _: U::Assoc) {}
fn _inner2<U: Trait>(self) -> U::Assoc { unimplemented!() }
}
}

fn main() {}

0 comments on commit 656ce3f

Please sign in to comment.