Rollup merge of #64250 - Xanewok:save-analysis-assoc-nested, r=varkor
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
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
index 25d921b..0155803 100644
--- a/src/librustc/ty/context.rs
+++ b/src/librustc/ty/context.rs
@@ -205,26 +205,24 @@
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")
}
}
}
diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
index 12c5ce1..55f6b91 100644
--- a/src/librustc_save_analysis/dump_visitor.rs
+++ b/src/librustc_save_analysis/dump_visitor.rs
@@ -130,6 +130,10 @@
self.save_ctxt.span_from_span(span)
}
+ fn lookup_def_id(&self, ref_id: NodeId) -> Option<DefId> {
+ self.save_ctxt.lookup_def_id(ref_id)
+ }
+
pub fn dump_crate_info(&mut self, name: &str, krate: &ast::Crate) {
let source_file = self.tcx.sess.local_crate_source_file.as_ref();
let crate_root = source_file.map(|source_file| {
@@ -223,13 +227,6 @@
}
}
- fn lookup_def_id(&self, ref_id: NodeId) -> Option<DefId> {
- match self.save_ctxt.get_path_res(ref_id) {
- Res::PrimTy(..) | Res::SelfTy(..) | Res::Err => None,
- def => Some(def.def_id()),
- }
- }
-
fn process_formals(&mut self, formals: &'l [ast::Param], qualname: &str) {
for arg in formals {
self.visit_pat(&arg.pat);
@@ -283,36 +280,32 @@
) {
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);
+
+ method_data.value = crate::make_signature(&sig.decl, &generics);
+ method_data.sig = sig::method_signature(id, ident, generics, sig, &v.save_ctxt);
+
+ v.dumper.dump_def(&access_from_vis!(v.save_ctxt, vis, hir_id), method_data);
}
- self.process_generic_params(&generics, &method_data.qualname, id);
+ // walk arg and return types
+ for arg in &sig.decl.inputs {
+ v.visit_ty(&arg.ty);
+ }
- 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);
- }
+ if let ast::FunctionRetTy::Ty(ref ret_ty) = sig.decl.output {
+ v.visit_ty(ret_ty);
+ }
- // walk arg and return types
- for arg in &sig.decl.inputs {
- self.visit_ty(&arg.ty);
- }
-
- if let ast::FunctionRetTy::Ty(ref ret_ty) = sig.decl.output {
- self.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) {
@@ -377,26 +370,31 @@
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 {
+ if let ast::TyKind::ImplTrait(..) = ret_ty.node {
+ // FIXME: Opaque type desugaring prevents us from easily
+ // processing trait bounds. See `visit_ty` for more details.
+ } else {
+ v.visit_ty(&ret_ty);
+ }
+ }
+
+ v.visit_block(&body);
+ });
}
fn process_static_or_const_item(
@@ -1113,11 +1111,7 @@
// FIXME: uses of the assoc type should ideally point to this
// 'def' and the name here should be a ref to the def in the
// trait.
- for bound in bounds.iter() {
- if let ast::GenericBound::Trait(trait_ref, _) = bound {
- self.process_path(trait_ref.trait_ref.ref_id, &trait_ref.trait_ref.path)
- }
- }
+ self.process_bounds(&bounds);
}
ast::ImplItemKind::Macro(_) => {}
}
@@ -1364,10 +1358,10 @@
self.visit_ty(&ty);
self.process_generic_params(ty_params, &qualname, item.id);
}
- OpaqueTy(ref _bounds, ref ty_params) => {
+ OpaqueTy(ref bounds, ref ty_params) => {
let qualname = format!("::{}",
self.tcx.def_path_str(self.tcx.hir().local_def_id_from_node_id(item.id)));
- // FIXME do something with _bounds
+
let value = String::new();
if !self.span.filter_generated(item.ident.span) {
let span = self.span_from_span(item.ident.span);
@@ -1393,6 +1387,7 @@
);
}
+ self.process_bounds(bounds);
self.process_generic_params(ty_params, &qualname, item.id);
}
Mac(_) => (),
@@ -1449,6 +1444,18 @@
self.visit_ty(element);
self.nest_tables(length.id, |v| v.visit_expr(&length.value));
}
+ ast::TyKind::ImplTrait(id, ref bounds) => {
+ // FIXME: As of writing, the opaque type lowering introduces
+ // another DefPath scope/segment (used to declare the resulting
+ // opaque type item).
+ // However, the synthetic scope does *not* have associated
+ // typeck tables, which means we can't nest it and we fire an
+ // assertion when resolving the qualified type paths in trait
+ // bounds...
+ // This will panic if called on return type `impl Trait`, which
+ // we guard against in `process_fn`.
+ self.nest_tables(id, |v| v.process_bounds(bounds));
+ }
_ => visit::walk_ty(self, t),
}
}
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
index 4bc098d..055ccf6 100644
--- a/src/librustc_save_analysis/lib.rs
+++ b/src/librustc_save_analysis/lib.rs
@@ -312,7 +312,7 @@
let impl_id = self.next_impl_id();
let span = self.span_from_span(sub_span);
- let type_data = self.lookup_ref_id(typ.id);
+ let type_data = self.lookup_def_id(typ.id);
type_data.map(|type_data| {
Data::RelationData(Relation {
kind: RelationKind::Impl {
@@ -322,7 +322,7 @@
from: id_from_def_id(type_data),
to: trait_ref
.as_ref()
- .and_then(|t| self.lookup_ref_id(t.ref_id))
+ .and_then(|t| self.lookup_def_id(t.ref_id))
.map(id_from_def_id)
.unwrap_or_else(|| null_id()),
},
@@ -495,7 +495,7 @@
}
pub fn get_trait_ref_data(&self, trait_ref: &ast::TraitRef) -> Option<Ref> {
- self.lookup_ref_id(trait_ref.ref_id).and_then(|def_id| {
+ self.lookup_def_id(trait_ref.ref_id).and_then(|def_id| {
let span = trait_ref.path.span;
if generated_code(span) {
return None;
@@ -870,7 +870,7 @@
})
}
- fn lookup_ref_id(&self, ref_id: NodeId) -> Option<DefId> {
+ fn lookup_def_id(&self, ref_id: NodeId) -> Option<DefId> {
match self.get_path_res(ref_id) {
Res::PrimTy(_) | Res::SelfTy(..) | Res::Err => None,
def => Some(def.def_id()),
diff --git a/src/test/ui/save-analysis/issue-63663.rs b/src/test/ui/save-analysis/issue-63663.rs
new file mode 100644
index 0000000..92e8588
--- /dev/null
+++ b/src/test/ui/save-analysis/issue-63663.rs
@@ -0,0 +1,28 @@
+// check-pass
+// compile-flags: -Zsave-analysis
+
+pub trait Trait {
+ type Assoc;
+}
+
+pub struct A;
+
+trait Generic<T> {}
+impl<T> Generic<T> for () {}
+
+// Don't ICE when resolving type paths in return type `impl Trait`
+fn assoc_in_opaque_type_bounds<U: Trait>() -> impl Generic<U::Assoc> {}
+
+// Check that this doesn't ICE when processing associated const in formal
+// argument and return type of functions defined inside function/method scope.
+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() {}
