Skip to content

Commit 81e2107

Browse files
compiler-errorscompiler-errors
committed
Dont use span as key when collecting missing associated types from dyn
1 parent c322cd5 commit 81e2107

File tree

2 files changed

+129
-143
lines changed

2 files changed

+129
-143
lines changed

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

+20-21
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
1-
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
1+
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
22
use rustc_errors::codes::*;
33
use rustc_errors::struct_span_code_err;
44
use rustc_hir as hir;
55
use rustc_hir::def::{DefKind, Res};
6-
use rustc_hir::def_id::DefId;
76
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
87
use rustc_middle::span_bug;
98
use rustc_middle::ty::fold::BottomUpFolder;
109
use rustc_middle::ty::{
1110
self, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
1211
TypeVisitableExt, Upcast,
1312
};
14-
use rustc_span::{ErrorGuaranteed, Span};
13+
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
1514
use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility;
1615
use rustc_trait_selection::traits::{self, hir_ty_lowering_dyn_compatibility_violations};
1716
use rustc_type_ir::elaborate::ClauseWithSupertraitSpan;
@@ -128,8 +127,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
128127
}
129128
}
130129

131-
let mut associated_types: FxIndexMap<Span, FxIndexSet<DefId>> = FxIndexMap::default();
130+
let mut needed_associated_types = FxIndexSet::default();
132131

132+
let principal_span = regular_traits.first().map_or(DUMMY_SP, |info| info.bottom().1);
133133
let regular_traits_refs_spans = trait_bounds
134134
.into_iter()
135135
.filter(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id()));
@@ -146,11 +146,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
146146
match bound_predicate.skip_binder() {
147147
ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => {
148148
let pred = bound_predicate.rebind(pred);
149-
associated_types.entry(original_span).or_default().extend(
149+
needed_associated_types.extend(
150150
tcx.associated_items(pred.def_id())
151151
.in_definition_order()
152152
.filter(|item| item.kind == ty::AssocKind::Type)
153153
.filter(|item| !item.is_impl_trait_in_trait())
154+
// If the associated type has a `where Self: Sized` bound,
155+
// we do not need to constrain the associated type.
156+
.filter(|item| !tcx.generics_require_sized_self(item.def_id))
154157
.map(|item| item.def_id),
155158
);
156159
}
@@ -201,26 +204,22 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
201204
// So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated
202205
// types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
203206
// corresponding `Projection` clause
204-
for def_ids in associated_types.values_mut() {
205-
for (projection_bound, span) in &projection_bounds {
206-
let def_id = projection_bound.projection_def_id();
207-
def_ids.swap_remove(&def_id);
208-
if tcx.generics_require_sized_self(def_id) {
209-
tcx.emit_node_span_lint(
210-
UNUSED_ASSOCIATED_TYPE_BOUNDS,
211-
hir_id,
212-
*span,
213-
crate::errors::UnusedAssociatedTypeBounds { span: *span },
214-
);
215-
}
207+
for (projection_bound, span) in &projection_bounds {
208+
let def_id = projection_bound.projection_def_id();
209+
needed_associated_types.swap_remove(&def_id);
210+
if tcx.generics_require_sized_self(def_id) {
211+
tcx.emit_node_span_lint(
212+
UNUSED_ASSOCIATED_TYPE_BOUNDS,
213+
hir_id,
214+
*span,
215+
crate::errors::UnusedAssociatedTypeBounds { span: *span },
216+
);
216217
}
217-
// If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated
218-
// type in the `dyn Trait`.
219-
def_ids.retain(|def_id| !tcx.generics_require_sized_self(def_id));
220218
}
221219

222220
self.complain_about_missing_assoc_tys(
223-
associated_types,
221+
principal_span,
222+
needed_associated_types,
224223
potential_assoc_types,
225224
hir_trait_bounds,
226225
);

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

+109-122
Original file line numberDiff line numberDiff line change
@@ -716,45 +716,35 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
716716
/// emit a generic note suggesting using a `where` clause to constraint instead.
717717
pub(crate) fn complain_about_missing_assoc_tys(
718718
&self,
719-
associated_types: FxIndexMap<Span, FxIndexSet<DefId>>,
719+
principal_span: Span,
720+
missing_assoc_types: FxIndexSet<DefId>,
720721
potential_assoc_types: Vec<usize>,
721722
trait_bounds: &[hir::PolyTraitRef<'_>],
722723
) {
723-
if associated_types.values().all(|v| v.is_empty()) {
724+
if missing_assoc_types.is_empty() {
724725
return;
725726
}
726727

727728
let tcx = self.tcx();
728-
// FIXME: Marked `mut` so that we can replace the spans further below with a more
729-
// appropriate one, but this should be handled earlier in the span assignment.
730-
let associated_types: FxIndexMap<Span, Vec<_>> = associated_types
731-
.into_iter()
732-
.map(|(span, def_ids)| {
733-
(span, def_ids.into_iter().map(|did| tcx.associated_item(did)).collect())
734-
})
735-
.collect();
729+
let missing_assoc_types: Vec<_> =
730+
missing_assoc_types.into_iter().map(|def_id| tcx.associated_item(def_id)).collect();
736731
let mut names: FxIndexMap<String, Vec<Symbol>> = Default::default();
737732
let mut names_len = 0;
738733

739734
// Account for things like `dyn Foo + 'a`, like in tests `issue-22434.rs` and
740735
// `issue-22560.rs`.
741-
let mut trait_bound_spans: Vec<Span> = vec![];
742736
let mut dyn_compatibility_violations = false;
743-
for (span, items) in &associated_types {
744-
if !items.is_empty() {
745-
trait_bound_spans.push(*span);
746-
}
747-
for assoc_item in items {
748-
let trait_def_id = assoc_item.container_id(tcx);
749-
names.entry(tcx.def_path_str(trait_def_id)).or_default().push(assoc_item.name);
750-
names_len += 1;
751-
752-
let violations =
753-
dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item);
754-
if !violations.is_empty() {
755-
report_dyn_incompatibility(tcx, *span, None, trait_def_id, &violations).emit();
756-
dyn_compatibility_violations = true;
757-
}
737+
for assoc_item in &missing_assoc_types {
738+
let trait_def_id = assoc_item.container_id(tcx);
739+
names.entry(tcx.def_path_str(trait_def_id)).or_default().push(assoc_item.name);
740+
names_len += 1;
741+
742+
let violations =
743+
dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item);
744+
if !violations.is_empty() {
745+
report_dyn_incompatibility(tcx, principal_span, None, trait_def_id, &violations)
746+
.emit();
747+
dyn_compatibility_violations = true;
758748
}
759749
}
760750
if dyn_compatibility_violations {
@@ -814,10 +804,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
814804
names.sort();
815805
let names = names.join(", ");
816806

817-
trait_bound_spans.sort();
818807
let mut err = struct_span_code_err!(
819808
self.dcx(),
820-
trait_bound_spans,
809+
principal_span,
821810
E0191,
822811
"the value of the associated type{} {} must be specified",
823812
pluralize!(names_len),
@@ -827,81 +816,81 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
827816
let mut types_count = 0;
828817
let mut where_constraints = vec![];
829818
let mut already_has_generics_args_suggestion = false;
830-
for (span, assoc_items) in &associated_types {
831-
let mut names: UnordMap<_, usize> = Default::default();
832-
for item in assoc_items {
833-
types_count += 1;
834-
*names.entry(item.name).or_insert(0) += 1;
835-
}
836-
let mut dupes = false;
837-
let mut shadows = false;
838-
for item in assoc_items {
839-
let prefix = if names[&item.name] > 1 {
840-
let trait_def_id = item.container_id(tcx);
841-
dupes = true;
842-
format!("{}::", tcx.def_path_str(trait_def_id))
843-
} else if bound_names.get(&item.name).is_some_and(|x| x != &item) {
844-
let trait_def_id = item.container_id(tcx);
845-
shadows = true;
846-
format!("{}::", tcx.def_path_str(trait_def_id))
847-
} else {
848-
String::new()
849-
};
850-
851-
let mut is_shadowed = false;
852819

853-
if let Some(assoc_item) = bound_names.get(&item.name)
854-
&& assoc_item != &item
855-
{
856-
is_shadowed = true;
820+
let mut names: UnordMap<_, usize> = Default::default();
821+
for item in &missing_assoc_types {
822+
types_count += 1;
823+
*names.entry(item.name).or_insert(0) += 1;
824+
}
825+
let mut dupes = false;
826+
let mut shadows = false;
827+
for item in &missing_assoc_types {
828+
let prefix = if names[&item.name] > 1 {
829+
let trait_def_id = item.container_id(tcx);
830+
dupes = true;
831+
format!("{}::", tcx.def_path_str(trait_def_id))
832+
} else if bound_names.get(&item.name).is_some_and(|x| *x != item) {
833+
let trait_def_id = item.container_id(tcx);
834+
shadows = true;
835+
format!("{}::", tcx.def_path_str(trait_def_id))
836+
} else {
837+
String::new()
838+
};
857839

858-
let rename_message =
859-
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
860-
err.span_label(
861-
tcx.def_span(assoc_item.def_id),
862-
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
863-
);
864-
}
840+
let mut is_shadowed = false;
865841

866-
let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
842+
if let Some(assoc_item) = bound_names.get(&item.name)
843+
&& *assoc_item != item
844+
{
845+
is_shadowed = true;
867846

868-
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
869-
err.span_label(
870-
sp,
871-
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
872-
);
873-
}
847+
let rename_message =
848+
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
849+
err.span_label(
850+
tcx.def_span(assoc_item.def_id),
851+
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
852+
);
874853
}
875-
if potential_assoc_types.len() == assoc_items.len() {
876-
// When the amount of missing associated types equals the number of
877-
// extra type arguments present. A suggesting to replace the generic args with
878-
// associated types is already emitted.
879-
already_has_generics_args_suggestion = true;
880-
} else if let (Ok(snippet), false, false) =
881-
(tcx.sess.source_map().span_to_snippet(*span), dupes, shadows)
882-
{
883-
let types: Vec<_> =
884-
assoc_items.iter().map(|item| format!("{} = Type", item.name)).collect();
885-
let code = if snippet.ends_with('>') {
886-
// The user wrote `Trait<'a>` or similar and we don't have a type we can
887-
// suggest, but at least we can clue them to the correct syntax
888-
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
889-
// suggestion.
890-
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
891-
} else if in_expr_or_pat {
892-
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
893-
// least we can clue them to the correct syntax `Iterator::<Item = Type>`.
894-
format!("{}::<{}>", snippet, types.join(", "))
895-
} else {
896-
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
897-
// least we can clue them to the correct syntax `Iterator<Item = Type>`.
898-
format!("{}<{}>", snippet, types.join(", "))
899-
};
900-
suggestions.push((*span, code));
901-
} else if dupes {
902-
where_constraints.push(*span);
854+
855+
let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
856+
857+
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
858+
err.span_label(
859+
sp,
860+
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
861+
);
903862
}
904863
}
864+
if potential_assoc_types.len() == missing_assoc_types.len() {
865+
// When the amount of missing associated types equals the number of
866+
// extra type arguments present. A suggesting to replace the generic args with
867+
// associated types is already emitted.
868+
already_has_generics_args_suggestion = true;
869+
} else if let (Ok(snippet), false, false) =
870+
(tcx.sess.source_map().span_to_snippet(principal_span), dupes, shadows)
871+
{
872+
let types: Vec<_> =
873+
missing_assoc_types.iter().map(|item| format!("{} = Type", item.name)).collect();
874+
let code = if snippet.ends_with('>') {
875+
// The user wrote `Trait<'a>` or similar and we don't have a type we can
876+
// suggest, but at least we can clue them to the correct syntax
877+
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
878+
// suggestion.
879+
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
880+
} else if in_expr_or_pat {
881+
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
882+
// least we can clue them to the correct syntax `Iterator::<Item = Type>`.
883+
format!("{}::<{}>", snippet, types.join(", "))
884+
} else {
885+
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
886+
// least we can clue them to the correct syntax `Iterator<Item = Type>`.
887+
format!("{}<{}>", snippet, types.join(", "))
888+
};
889+
suggestions.push((principal_span, code));
890+
} else if dupes {
891+
where_constraints.push(principal_span);
892+
}
893+
905894
let where_msg = "consider introducing a new type parameter, adding `where` constraints \
906895
using the fully-qualified path to the associated types";
907896
if !where_constraints.is_empty() && suggestions.is_empty() {
@@ -912,32 +901,30 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
912901
}
913902
if suggestions.len() != 1 || already_has_generics_args_suggestion {
914903
// We don't need this label if there's an inline suggestion, show otherwise.
915-
for (span, assoc_items) in &associated_types {
916-
let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
917-
for item in assoc_items {
918-
types_count += 1;
919-
*names.entry(item.name).or_insert(0) += 1;
920-
}
921-
let mut label = vec![];
922-
for item in assoc_items {
923-
let postfix = if names[&item.name] > 1 {
924-
let trait_def_id = item.container_id(tcx);
925-
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
926-
} else {
927-
String::new()
928-
};
929-
label.push(format!("`{}`{}", item.name, postfix));
930-
}
931-
if !label.is_empty() {
932-
err.span_label(
933-
*span,
934-
format!(
935-
"associated type{} {} must be specified",
936-
pluralize!(label.len()),
937-
label.join(", "),
938-
),
939-
);
940-
}
904+
let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
905+
for item in &missing_assoc_types {
906+
types_count += 1;
907+
*names.entry(item.name).or_insert(0) += 1;
908+
}
909+
let mut label = vec![];
910+
for item in &missing_assoc_types {
911+
let postfix = if names[&item.name] > 1 {
912+
let trait_def_id = item.container_id(tcx);
913+
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
914+
} else {
915+
String::new()
916+
};
917+
label.push(format!("`{}`{}", item.name, postfix));
918+
}
919+
if !label.is_empty() {
920+
err.span_label(
921+
principal_span,
922+
format!(
923+
"associated type{} {} must be specified",
924+
pluralize!(label.len()),
925+
label.join(", "),
926+
),
927+
);
941928
}
942929
}
943930
suggestions.sort_by_key(|&(span, _)| span);

0 commit comments

Comments
 (0)