Skip to content

Commit

Permalink
Auto merge of #92441 - cjgillot:resolve-trait-impl-item, r=matthewjasper
Browse files Browse the repository at this point in the history
Link impl items to corresponding trait items in late resolver.

Hygienically linking trait impl items to declarations in the trait can be done directly by the late resolver. In fact, it is already done to diagnose unknown items.

This PR uses this resolution work and stores the `DefId` of the trait item in the HIR. This avoids having to do this resolution manually later.

r? `@matthewjasper`
Related to #90639. The added `trait_item_id` field can be moved to `ImplItemRef` to be used directly by your PR.
  • Loading branch information
bors committed Jan 15, 2022
2 parents b13a5bf + 441c1a6 commit ec4bcaa
Show file tree
Hide file tree
Showing 26 changed files with 474 additions and 511 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2881,6 +2881,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<DefId>,
}

#[derive(Copy, Clone, PartialEq, Encodable, Debug, HashStable_Generic)]
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_middle/src/ty/assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &'a ty::AssocItem> {
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,
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,25 @@ impl<'a> Resolver<'a> {

err
}
ResolutionError::TraitImplMismatch {
name,
kind,
code,
trait_item_span,
trait_path,
} => {
let mut err = self.session.struct_span_err_with_code(
span,
&format!(
"item `{}` is an associated {}, which doesn't match its trait `{}`",
name, kind, trait_path,
),
code,
);
err.span_label(span, "does not match trait");
err.span_label(trait_item_span, "item in trait");
err
}
}
}

Expand Down
85 changes: 62 additions & 23 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1247,15 +1247,14 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
);
let res = res.base_res();
if res != Res::Err {
new_id = Some(res.def_id());
let span = trait_ref.path.span;
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) = self.resolve_path(
&path,
Some(TypeNS),
false,
span,
true,
trait_ref.path.span,
CrateLint::SimplePath(trait_ref.ref_id),
) {
new_id = Some(res.def_id());
new_val = Some((module, trait_ref.clone()));
}
}
Expand Down Expand Up @@ -1324,6 +1323,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,
Expand Down Expand Up @@ -1359,6 +1359,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,
Expand Down Expand Up @@ -1386,6 +1387,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,
Expand Down Expand Up @@ -1416,34 +1418,71 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {

fn check_trait_item<F>(
&mut self,
ident: Ident,
id: NodeId,
mut ident: Ident,
kind: &AssocItemKind,
ns: Namespace,
span: Span,
err: F,
) where
F: FnOnce(Ident, &str, Option<Symbol>) -> 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; };
ident.span.normalize_to_macros_2_0_and_adjust(module.expansion);
let key = self.r.new_key(ident, ns);
let mut binding = self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.binding);
debug!(?binding);
if binding.is_none() {
// 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,
};
let key = self.r.new_key(ident, ns);
binding = self.r.resolution(module, key).try_borrow().ok().and_then(|r| r.binding);
debug!(?binding);
}
let Some(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 (code, kind) = match kind {
AssocItemKind::Const(..) => (rustc_errors::error_code!(E0323), "const"),
AssocItemKind::Fn(..) => (rustc_errors::error_code!(E0324), "method"),
AssocItemKind::TyAlias(..) => (rustc_errors::error_code!(E0325), "type"),
AssocItemKind::MacCall(..) => span_bug!(span, "unexpanded macro"),
};
self.report_error(
span,
ResolutionError::TraitImplMismatch {
name: ident.name,
kind,
code,
trait_path: path_names_to_string(path),
trait_item_span: binding.span,
},
);
}

fn resolve_params(&mut self, params: &'ast [Param]) {
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ enum ResolutionError<'a> {
SelfInGenericParamDefault,
/// Error E0767: use of unreachable label
UnreachableLabel { name: Symbol, definition_span: Span, suggestion: Option<LabelSuggestion> },
/// Error E0323, E0324, E0325: mismatch between trait item and impl item.
TraitImplMismatch {
name: Symbol,
kind: &'static str,
trait_path: String,
trait_item_span: Span,
code: rustc_errors::DiagnosticId,
},
}

enum VisResolutionError<'a> {
Expand Down
106 changes: 2 additions & 104 deletions compiler/rustc_ty_utils/src/assoc.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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<DefId> {
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();
}
Loading

0 comments on commit ec4bcaa

Please sign in to comment.