From 1be6e2d6e9df7de641b554987f87c775c1e37d18 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 8 Jan 2022 12:22:06 +0100 Subject: [PATCH] Link impl items to corresponding trait items in late resolver. --- compiler/rustc_ast_lowering/src/index.rs | 3 +- compiler/rustc_ast_lowering/src/item.rs | 1 + compiler/rustc_hir/src/hir.rs | 2 + compiler/rustc_hir/src/intravisit.rs | 3 +- compiler/rustc_middle/src/ty/assoc.rs | 15 --- compiler/rustc_resolve/src/late.rs | 110 +++++++++++++++--- compiler/rustc_ty_utils/src/assoc.rs | 106 +---------------- src/test/ui/span/impl-wrong-item-for-trait.rs | 8 +- .../ui/span/impl-wrong-item-for-trait.stderr | 28 ++--- 9 files changed, 114 insertions(+), 162 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/index.rs b/compiler/rustc_ast_lowering/src/index.rs index 8a9dad2cdd7..39a8cd405de 100644 --- a/compiler/rustc_ast_lowering/src/index.rs +++ b/compiler/rustc_ast_lowering/src/index.rs @@ -335,7 +335,8 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { fn visit_impl_item_ref(&mut self, ii: &'hir ImplItemRef) { // Do not visit the duplicate information in ImplItemRef. We want to // map the actual nodes, not the duplicate ones in the *Ref. - let ImplItemRef { id, ident: _, kind: _, span: _, defaultness: _ } = *ii; + let ImplItemRef { id, ident: _, kind: _, span: _, defaultness: _, trait_item_def_id: _ } = + *ii; self.visit_nested_impl_item(id); } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 92cae4da89a..ed3abbd5b4d 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -925,6 +925,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } AssocItemKind::MacCall(..) => unimplemented!(), }, + trait_item_def_id: self.resolver.get_partial_res(i.id).map(|r| r.base_res().def_id()), } } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index d59756239d9..bcf677bbafd 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2885,6 +2885,8 @@ pub struct ImplItemRef { pub kind: AssocItemKind, pub span: Span, pub defaultness: Defaultness, + /// When we are in a trait impl, link to the trait-item's id. + pub trait_item_def_id: Option, } #[derive(Copy, Clone, PartialEq, Encodable, Debug, HashStable_Generic)] diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index d0eee422202..7c77930193c 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -1088,7 +1088,8 @@ pub fn walk_foreign_item_ref<'v, V: Visitor<'v>>( pub fn walk_impl_item_ref<'v, V: Visitor<'v>>(visitor: &mut V, impl_item_ref: &'v ImplItemRef) { // N.B., deliberately force a compilation error if/when new fields are added. - let ImplItemRef { id, ident, ref kind, span: _, ref defaultness } = *impl_item_ref; + let ImplItemRef { id, ident, ref kind, span: _, ref defaultness, trait_item_def_id: _ } = + *impl_item_ref; visitor.visit_nested_impl_item(id); visitor.visit_ident(ident); visitor.visit_associated_item_kind(kind); diff --git a/compiler/rustc_middle/src/ty/assoc.rs b/compiler/rustc_middle/src/ty/assoc.rs index 5af4eef40d4..8563bac0bbf 100644 --- a/compiler/rustc_middle/src/ty/assoc.rs +++ b/compiler/rustc_middle/src/ty/assoc.rs @@ -139,21 +139,6 @@ impl<'tcx> AssocItems<'tcx> { self.items.get_by_key(name).copied() } - /// Returns an iterator over all associated items with the given name. - /// - /// Multiple items may have the same name if they are in different `Namespace`s. For example, - /// an associated type can have the same name as a method. Use one of the `find_by_name_and_*` - /// methods below if you know which item you are looking for. - pub fn filter_by_name<'a>( - &'a self, - tcx: TyCtxt<'a>, - ident: Ident, - parent_def_id: DefId, - ) -> impl 'a + Iterator { - self.filter_by_name_unhygienic(ident.name) - .filter(move |item| tcx.hygienic_eq(ident, item.ident, parent_def_id)) - } - /// Returns the associated item with the given name and `AssocKind`, if one exists. pub fn find_by_name_and_kind( &self, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 57305023138..24ff04b8853 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1317,6 +1317,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // If this is a trait impl, ensure the const // exists in trait this.check_trait_item( + item.id, item.ident, &item.kind, ValueNS, @@ -1352,6 +1353,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // If this is a trait impl, ensure the method // exists in trait this.check_trait_item( + item.id, item.ident, &item.kind, ValueNS, @@ -1379,6 +1381,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // If this is a trait impl, ensure the type // exists in trait this.check_trait_item( + item.id, item.ident, &item.kind, TypeNS, @@ -1409,6 +1412,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { fn check_trait_item( &mut self, + id: NodeId, ident: Ident, kind: &AssocItemKind, ns: Namespace, @@ -1417,26 +1421,94 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ) where F: FnOnce(Ident, &str, Option) -> ResolutionError<'_>, { - // If there is a TraitRef in scope for an impl, then the method must be in the - // trait. - if let Some((module, _)) = self.current_trait_ref { - if self - .r - .resolve_ident_in_module( - ModuleOrUniformRoot::Module(module), - ident, - ns, - &self.parent_scope, - false, - span, - ) - .is_err() - { - let candidate = self.find_similarly_named_assoc_item(ident.name, kind); - let path = &self.current_trait_ref.as_ref().unwrap().1.path; - self.report_error(span, err(ident, &path_names_to_string(path), candidate)); - } + // If there is a TraitRef in scope for an impl, then the method must be in the trait. + let Some((module, _)) = &self.current_trait_ref else { return; }; + let mut binding = self.r.resolve_ident_in_module( + ModuleOrUniformRoot::Module(module), + ident, + ns, + &self.parent_scope, + false, + span, + ); + if binding.is_err() { + // We could not find the trait item in the correct namespace. + // Check the other namespace to report an error. + let ns = match ns { + ValueNS => TypeNS, + TypeNS => ValueNS, + _ => ns, + }; + binding = self.r.resolve_ident_in_module( + ModuleOrUniformRoot::Module(module), + ident, + ns, + &self.parent_scope, + false, + span, + ); } + let Ok(binding) = binding else { + // We could not find the method: report an error. + let candidate = self.find_similarly_named_assoc_item(ident.name, kind); + let path = &self.current_trait_ref.as_ref().unwrap().1.path; + self.report_error(span, err(ident, &path_names_to_string(path), candidate)); + return; + }; + + let res = binding.res(); + let Res::Def(def_kind, _) = res else { bug!() }; + match (def_kind, kind) { + (DefKind::AssocTy, AssocItemKind::TyAlias(..)) + | (DefKind::AssocFn, AssocItemKind::Fn(..)) + | (DefKind::AssocConst, AssocItemKind::Const(..)) => { + self.r.record_partial_res(id, PartialRes::new(res)); + return; + } + _ => {} + } + + // The method kind does not correspond to what appeared in the trait, report. + let path = &self.current_trait_ref.as_ref().unwrap().1.path; + let path = &path_names_to_string(path); + let mut err = match kind { + AssocItemKind::Const(..) => { + rustc_errors::struct_span_err!( + self.r.session, + span, + E0323, + "item `{}` is an associated const, which doesn't match its trait `{}`", + ident, + path, + ) + } + AssocItemKind::Fn(..) => { + rustc_errors::struct_span_err!( + self.r.session, + span, + E0324, + "item `{}` is an associated method, which doesn't match its trait `{}`", + ident, + path, + ) + } + AssocItemKind::TyAlias(..) => { + rustc_errors::struct_span_err!( + self.r.session, + span, + E0325, + "item `{}` is an associated type, which doesn't match its trait `{}`", + ident, + path, + ) + } + AssocItemKind::MacCall(..) => { + span_bug!(span, "macros should have been expanded") + } + }; + err.span_label(span, "does not match trait"); + err.span_label(binding.span, "item in trait"); + err.emit(); } fn resolve_params(&mut self, params: &'ast [Param]) { diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs index b1d47f6c29a..6e2ef27f108 100644 --- a/compiler/rustc_ty_utils/src/assoc.rs +++ b/compiler/rustc_ty_utils/src/assoc.rs @@ -1,8 +1,7 @@ use rustc_data_structures::fx::FxHashMap; -use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, TyCtxt}; pub fn provide(providers: &mut ty::query::Providers) { *providers = ty::query::Providers { @@ -125,115 +124,14 @@ fn associated_item_from_impl_item_ref( hir::AssocItemKind::Type => (ty::AssocKind::Type, false), }; - let trait_item_def_id = impl_item_base_id(tcx, parent_def_id, impl_item_ref); - ty::AssocItem { ident: impl_item_ref.ident, kind, vis: tcx.visibility(def_id), defaultness: impl_item_ref.defaultness, def_id: def_id.to_def_id(), - trait_item_def_id, + trait_item_def_id: impl_item_ref.trait_item_def_id, container: ty::ImplContainer(parent_def_id.to_def_id()), fn_has_self_parameter: has_self, } } - -fn impl_item_base_id<'tcx>( - tcx: TyCtxt<'tcx>, - parent_def_id: LocalDefId, - impl_item: &hir::ImplItemRef, -) -> Option { - let impl_trait_ref = tcx.impl_trait_ref(parent_def_id)?; - - // If the trait reference itself is erroneous (so the compilation is going - // to fail), skip checking the items here -- the `impl_item` table in `tcx` - // isn't populated for such impls. - if impl_trait_ref.references_error() { - return None; - } - - // Locate trait items - let associated_items = tcx.associated_items(impl_trait_ref.def_id); - - // Match item against trait - let mut items = associated_items.filter_by_name(tcx, impl_item.ident, impl_trait_ref.def_id); - - let mut trait_item = items.next()?; - - let is_compatible = |ty: &&ty::AssocItem| match (ty.kind, &impl_item.kind) { - (ty::AssocKind::Const, hir::AssocItemKind::Const) => true, - (ty::AssocKind::Fn, hir::AssocItemKind::Fn { .. }) => true, - (ty::AssocKind::Type, hir::AssocItemKind::Type) => true, - _ => false, - }; - - // If we don't have a compatible item, we'll use the first one whose name matches - // to report an error. - let mut compatible_kind = is_compatible(&trait_item); - - if !compatible_kind { - if let Some(ty_trait_item) = items.find(is_compatible) { - compatible_kind = true; - trait_item = ty_trait_item; - } - } - - if compatible_kind { - Some(trait_item.def_id) - } else { - report_mismatch_error(tcx, trait_item.def_id, impl_trait_ref, impl_item); - None - } -} - -#[inline(never)] -#[cold] -fn report_mismatch_error<'tcx>( - tcx: TyCtxt<'tcx>, - trait_item_def_id: DefId, - impl_trait_ref: ty::TraitRef<'tcx>, - impl_item: &hir::ImplItemRef, -) { - let mut err = match impl_item.kind { - hir::AssocItemKind::Const => { - // Find associated const definition. - struct_span_err!( - tcx.sess, - impl_item.span, - E0323, - "item `{}` is an associated const, which doesn't match its trait `{}`", - impl_item.ident, - impl_trait_ref.print_only_trait_path() - ) - } - - hir::AssocItemKind::Fn { .. } => { - struct_span_err!( - tcx.sess, - impl_item.span, - E0324, - "item `{}` is an associated method, which doesn't match its trait `{}`", - impl_item.ident, - impl_trait_ref.print_only_trait_path() - ) - } - - hir::AssocItemKind::Type => { - struct_span_err!( - tcx.sess, - impl_item.span, - E0325, - "item `{}` is an associated type, which doesn't match its trait `{}`", - impl_item.ident, - impl_trait_ref.print_only_trait_path() - ) - } - }; - - err.span_label(impl_item.span, "does not match trait"); - if let Some(trait_span) = tcx.hir().span_if_local(trait_item_def_id) { - err.span_label(trait_span, "item in trait"); - } - err.emit(); -} diff --git a/src/test/ui/span/impl-wrong-item-for-trait.rs b/src/test/ui/span/impl-wrong-item-for-trait.rs index 672fe8dccf7..bf335868643 100644 --- a/src/test/ui/span/impl-wrong-item-for-trait.rs +++ b/src/test/ui/span/impl-wrong-item-for-trait.rs @@ -29,12 +29,10 @@ impl Foo for FooTypeForMethod { //~^ ERROR E0046 type bar = u64; //~^ ERROR E0325 - //~| ERROR E0437 const MY_CONST: u32 = 1; } -impl Debug for FooTypeForMethod { -} -//~^^ ERROR E0046 +impl Debug for FooTypeForMethod {} +//~^ ERROR E0046 -fn main () {} +fn main() {} diff --git a/src/test/ui/span/impl-wrong-item-for-trait.stderr b/src/test/ui/span/impl-wrong-item-for-trait.stderr index d805bbc7926..82ef13f3362 100644 --- a/src/test/ui/span/impl-wrong-item-for-trait.stderr +++ b/src/test/ui/span/impl-wrong-item-for-trait.stderr @@ -1,8 +1,11 @@ -error[E0437]: type `bar` is not a member of trait `Foo` - --> $DIR/impl-wrong-item-for-trait.rs:30:5 +error[E0323]: item `bar` is an associated const, which doesn't match its trait `Foo` + --> $DIR/impl-wrong-item-for-trait.rs:12:5 | -LL | type bar = u64; - | ^^^^^^^^^^^^^^^ not a member of trait `Foo` +LL | fn bar(&self); + | -------------- item in trait +... +LL | const bar: u64 = 1; + | ^^^^^^^^^^^^^^^^^^^ does not match trait error[E0324]: item `MY_CONST` is an associated method, which doesn't match its trait `Foo` --> $DIR/impl-wrong-item-for-trait.rs:22:5 @@ -13,15 +16,6 @@ LL | const MY_CONST: u32; LL | fn MY_CONST() {} | ^^^^^^^^^^^^^^^^ does not match trait -error[E0323]: item `bar` is an associated const, which doesn't match its trait `Foo` - --> $DIR/impl-wrong-item-for-trait.rs:12:5 - | -LL | fn bar(&self); - | -------------- item in trait -... -LL | const bar: u64 = 1; - | ^^^^^^^^^^^^^^^^^^^ does not match trait - error[E0325]: item `bar` is an associated type, which doesn't match its trait `Foo` --> $DIR/impl-wrong-item-for-trait.rs:30:5 | @@ -59,14 +53,14 @@ LL | impl Foo for FooTypeForMethod { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `bar` in implementation error[E0046]: not all trait items implemented, missing: `fmt` - --> $DIR/impl-wrong-item-for-trait.rs:36:1 + --> $DIR/impl-wrong-item-for-trait.rs:35:1 | -LL | impl Debug for FooTypeForMethod { +LL | impl Debug for FooTypeForMethod {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `fmt` in implementation | = help: implement the missing item: `fn fmt(&self, _: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { todo!() }` -error: aborting due to 8 previous errors +error: aborting due to 7 previous errors -Some errors have detailed explanations: E0046, E0323, E0324, E0325, E0437. +Some errors have detailed explanations: E0046, E0323, E0324, E0325. For more information about an error, try `rustc --explain E0046`.