Skip to content

Commit 0c6da32

Browse files
compiler-errorscompiler-errors
committed
Dont use span as key when collecting missing associated types from dyn
1 parent ea75aca commit 0c6da32

File tree

2 files changed

+130
-148
lines changed

2 files changed

+130
-148
lines changed

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

Lines changed: 20 additions & 21 deletions
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

Lines changed: 110 additions & 127 deletions
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+
mut 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 mut 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 {
@@ -794,11 +784,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
794784
// | ^^^^^^^^^^^^^ help: specify the associated type:
795785
// | `BitXor::bitor<Output = Type>`
796786
[segment] if segment.args.is_none() => {
797-
trait_bound_spans = vec![segment.ident.span];
798-
associated_types = associated_types
799-
.into_values()
800-
.map(|items| (segment.ident.span, items))
801-
.collect();
787+
principal_span = segment.ident.span;
802788
}
803789
_ => {}
804790
}
@@ -847,10 +833,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
847833
names.sort();
848834
let names = names.join(", ");
849835

850-
trait_bound_spans.sort();
851836
let mut err = struct_span_code_err!(
852837
self.dcx(),
853-
trait_bound_spans,
838+
principal_span,
854839
E0191,
855840
"the value of the associated type{} {} must be specified",
856841
pluralize!(names_len),
@@ -860,81 +845,81 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
860845
let mut types_count = 0;
861846
let mut where_constraints = vec![];
862847
let mut already_has_generics_args_suggestion = false;
863-
for (span, assoc_items) in &associated_types {
864-
let mut names: UnordMap<_, usize> = Default::default();
865-
for item in assoc_items {
866-
types_count += 1;
867-
*names.entry(item.name).or_insert(0) += 1;
868-
}
869-
let mut dupes = false;
870-
let mut shadows = false;
871-
for item in assoc_items {
872-
let prefix = if names[&item.name] > 1 {
873-
let trait_def_id = item.container_id(tcx);
874-
dupes = true;
875-
format!("{}::", tcx.def_path_str(trait_def_id))
876-
} else if bound_names.get(&item.name).is_some_and(|x| x != &item) {
877-
let trait_def_id = item.container_id(tcx);
878-
shadows = true;
879-
format!("{}::", tcx.def_path_str(trait_def_id))
880-
} else {
881-
String::new()
882-
};
883-
884-
let mut is_shadowed = false;
885848

886-
if let Some(assoc_item) = bound_names.get(&item.name)
887-
&& assoc_item != &item
888-
{
889-
is_shadowed = true;
849+
let mut names: UnordMap<_, usize> = Default::default();
850+
for item in &missing_assoc_types {
851+
types_count += 1;
852+
*names.entry(item.name).or_insert(0) += 1;
853+
}
854+
let mut dupes = false;
855+
let mut shadows = false;
856+
for item in &missing_assoc_types {
857+
let prefix = if names[&item.name] > 1 {
858+
let trait_def_id = item.container_id(tcx);
859+
dupes = true;
860+
format!("{}::", tcx.def_path_str(trait_def_id))
861+
} else if bound_names.get(&item.name).is_some_and(|x| *x != item) {
862+
let trait_def_id = item.container_id(tcx);
863+
shadows = true;
864+
format!("{}::", tcx.def_path_str(trait_def_id))
865+
} else {
866+
String::new()
867+
};
890868

891-
let rename_message =
892-
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
893-
err.span_label(
894-
tcx.def_span(assoc_item.def_id),
895-
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
896-
);
897-
}
869+
let mut is_shadowed = false;
898870

899-
let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
871+
if let Some(assoc_item) = bound_names.get(&item.name)
872+
&& *assoc_item != item
873+
{
874+
is_shadowed = true;
900875

901-
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
902-
err.span_label(
903-
sp,
904-
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
905-
);
906-
}
876+
let rename_message =
877+
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
878+
err.span_label(
879+
tcx.def_span(assoc_item.def_id),
880+
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
881+
);
907882
}
908-
if potential_assoc_types.len() == assoc_items.len() {
909-
// When the amount of missing associated types equals the number of
910-
// extra type arguments present. A suggesting to replace the generic args with
911-
// associated types is already emitted.
912-
already_has_generics_args_suggestion = true;
913-
} else if let (Ok(snippet), false, false) =
914-
(tcx.sess.source_map().span_to_snippet(*span), dupes, shadows)
915-
{
916-
let types: Vec<_> =
917-
assoc_items.iter().map(|item| format!("{} = Type", item.name)).collect();
918-
let code = if snippet.ends_with('>') {
919-
// The user wrote `Trait<'a>` or similar and we don't have a type we can
920-
// suggest, but at least we can clue them to the correct syntax
921-
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
922-
// suggestion.
923-
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
924-
} else if in_expr_or_pat {
925-
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
926-
// least we can clue them to the correct syntax `Iterator::<Item = Type>`.
927-
format!("{}::<{}>", snippet, types.join(", "))
928-
} else {
929-
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
930-
// least we can clue them to the correct syntax `Iterator<Item = Type>`.
931-
format!("{}<{}>", snippet, types.join(", "))
932-
};
933-
suggestions.push((*span, code));
934-
} else if dupes {
935-
where_constraints.push(*span);
883+
884+
let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
885+
886+
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
887+
err.span_label(
888+
sp,
889+
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
890+
);
936891
}
937892
}
893+
if potential_assoc_types.len() == missing_assoc_types.len() {
894+
// When the amount of missing associated types equals the number of
895+
// extra type arguments present. A suggesting to replace the generic args with
896+
// associated types is already emitted.
897+
already_has_generics_args_suggestion = true;
898+
} else if let (Ok(snippet), false, false) =
899+
(tcx.sess.source_map().span_to_snippet(principal_span), dupes, shadows)
900+
{
901+
let types: Vec<_> =
902+
missing_assoc_types.iter().map(|item| format!("{} = Type", item.name)).collect();
903+
let code = if snippet.ends_with('>') {
904+
// The user wrote `Trait<'a>` or similar and we don't have a type we can
905+
// suggest, but at least we can clue them to the correct syntax
906+
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
907+
// suggestion.
908+
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
909+
} else if in_expr_or_pat {
910+
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
911+
// least we can clue them to the correct syntax `Iterator::<Item = Type>`.
912+
format!("{}::<{}>", snippet, types.join(", "))
913+
} else {
914+
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
915+
// least we can clue them to the correct syntax `Iterator<Item = Type>`.
916+
format!("{}<{}>", snippet, types.join(", "))
917+
};
918+
suggestions.push((principal_span, code));
919+
} else if dupes {
920+
where_constraints.push(principal_span);
921+
}
922+
938923
let where_msg = "consider introducing a new type parameter, adding `where` constraints \
939924
using the fully-qualified path to the associated types";
940925
if !where_constraints.is_empty() && suggestions.is_empty() {
@@ -945,32 +930,30 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
945930
}
946931
if suggestions.len() != 1 || already_has_generics_args_suggestion {
947932
// We don't need this label if there's an inline suggestion, show otherwise.
948-
for (span, assoc_items) in &associated_types {
949-
let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
950-
for item in assoc_items {
951-
types_count += 1;
952-
*names.entry(item.name).or_insert(0) += 1;
953-
}
954-
let mut label = vec![];
955-
for item in assoc_items {
956-
let postfix = if names[&item.name] > 1 {
957-
let trait_def_id = item.container_id(tcx);
958-
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
959-
} else {
960-
String::new()
961-
};
962-
label.push(format!("`{}`{}", item.name, postfix));
963-
}
964-
if !label.is_empty() {
965-
err.span_label(
966-
*span,
967-
format!(
968-
"associated type{} {} must be specified",
969-
pluralize!(label.len()),
970-
label.join(", "),
971-
),
972-
);
973-
}
933+
let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
934+
for item in &missing_assoc_types {
935+
types_count += 1;
936+
*names.entry(item.name).or_insert(0) += 1;
937+
}
938+
let mut label = vec![];
939+
for item in &missing_assoc_types {
940+
let postfix = if names[&item.name] > 1 {
941+
let trait_def_id = item.container_id(tcx);
942+
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
943+
} else {
944+
String::new()
945+
};
946+
label.push(format!("`{}`{}", item.name, postfix));
947+
}
948+
if !label.is_empty() {
949+
err.span_label(
950+
principal_span,
951+
format!(
952+
"associated type{} {} must be specified",
953+
pluralize!(label.len()),
954+
label.join(", "),
955+
),
956+
);
974957
}
975958
}
976959
suggestions.sort_by_key(|&(span, _)| span);

0 commit comments

Comments
 (0)