Skip to content

Commit

Permalink
Add logic to make IMPLIED_BOUNDS_ENTAILMENT easier to understand
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Jan 13, 2023
1 parent 5457140 commit eaa7cc8
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 20 deletions.
160 changes: 148 additions & 12 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use super::potentially_plural_count;
use crate::errors::LifetimesOrBoundsMismatchOnTrait;
use hir::def_id::{DefId, LocalDefId};
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
use rustc_errors::{
pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed, MultiSpan,
};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit;
Expand Down Expand Up @@ -320,15 +322,6 @@ fn compare_method_predicate_entailment<'tcx>(
ty::Binder::dummy(ty::PredicateKind::WellFormed(unnormalized_impl_fty.into())),
));
}
let emit_implied_wf_lint = || {
infcx.tcx.struct_span_lint_hir(
rustc_session::lint::builtin::IMPLIED_BOUNDS_ENTAILMENT,
impl_m_hir_id,
infcx.tcx.def_span(impl_m.def_id),
"impl method assumes more implied bounds than the corresponding trait method",
|lint| lint,
);
};

// Check that all obligations are satisfied by the implementation's
// version.
Expand All @@ -346,7 +339,7 @@ fn compare_method_predicate_entailment<'tcx>(
)
.map(|()| {
// If the skip-mode was successful, emit a lint.
emit_implied_wf_lint();
emit_implied_wf_lint(infcx.tcx, impl_m, impl_m_hir_id, vec![]);
});
}
CheckImpliedWfMode::Skip => {
Expand Down Expand Up @@ -382,8 +375,16 @@ fn compare_method_predicate_entailment<'tcx>(
CheckImpliedWfMode::Skip,
)
.map(|()| {
let bad_args = extract_bad_args_for_implies_lint(
tcx,
&errors,
(trait_m, trait_sig),
// Unnormalized impl sig corresponds to the HIR types written
(impl_m, unnormalized_impl_sig),
impl_m_hir_id,
);
// If the skip-mode was successful, emit a lint.
emit_implied_wf_lint();
emit_implied_wf_lint(tcx, impl_m, impl_m_hir_id, bad_args);
});
}
CheckImpliedWfMode::Skip => {
Expand All @@ -400,6 +401,141 @@ fn compare_method_predicate_entailment<'tcx>(
Ok(())
}

fn extract_bad_args_for_implies_lint<'tcx>(
tcx: TyCtxt<'tcx>,
errors: &[infer::RegionResolutionError<'tcx>],
(trait_m, trait_sig): (&ty::AssocItem, ty::FnSig<'tcx>),
(impl_m, impl_sig): (&ty::AssocItem, ty::FnSig<'tcx>),
hir_id: hir::HirId,
) -> Vec<(Span, Option<String>)> {
let mut blame_generics = vec![];
for error in errors {
// Look for the subregion origin that contains an input/output type
let origin = match error {
infer::RegionResolutionError::ConcreteFailure(o, ..) => o,
infer::RegionResolutionError::GenericBoundFailure(o, ..) => o,
infer::RegionResolutionError::SubSupConflict(_, _, o, ..) => o,
infer::RegionResolutionError::UpperBoundUniverseConflict(.., o, _) => o,
};
// Extract (possible) input/output types from origin
match origin {
infer::SubregionOrigin::Subtype(trace) => {
if let Some((a, b)) = trace.values.ty() {
blame_generics.extend([a, b]);
}
}
infer::SubregionOrigin::RelateParamBound(_, ty, _) => blame_generics.push(*ty),
infer::SubregionOrigin::ReferenceOutlivesReferent(ty, _) => blame_generics.push(*ty),
_ => {}
}
}

let fn_decl = tcx.hir().fn_decl_by_hir_id(hir_id).unwrap();
let opt_ret_ty = match fn_decl.output {
hir::FnRetTy::DefaultReturn(_) => None,
hir::FnRetTy::Return(ty) => Some(ty),
};

// Map late-bound regions from trait to impl, so the names are right.
let mapping = std::iter::zip(
tcx.fn_sig(trait_m.def_id).bound_vars(),
tcx.fn_sig(impl_m.def_id).bound_vars(),
)
.filter_map(|(impl_bv, trait_bv)| {
if let ty::BoundVariableKind::Region(impl_bv) = impl_bv
&& let ty::BoundVariableKind::Region(trait_bv) = trait_bv
{
Some((impl_bv, trait_bv))
} else {
None
}
})
.collect();

// For each arg, see if it was in the "blame" of any of the region errors.
// If so, then try to produce a suggestion to replace the argument type with
// one from the trait.
let mut bad_args = vec![];
for (idx, (ty, hir_ty)) in
std::iter::zip(impl_sig.inputs_and_output, fn_decl.inputs.iter().chain(opt_ret_ty))
.enumerate()
{
let expected_ty = trait_sig.inputs_and_output[idx]
.fold_with(&mut RemapLateBound { tcx, mapping: &mapping });
if blame_generics.iter().any(|blame| ty.contains(*blame)) {
let expected_ty_sugg = expected_ty.to_string();
bad_args.push((
hir_ty.span,
// Only suggest something if it actually changed.
(expected_ty_sugg != ty.to_string()).then_some(expected_ty_sugg),
));
}
}

bad_args
}

struct RemapLateBound<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
mapping: &'a FxHashMap<ty::BoundRegionKind, ty::BoundRegionKind>,
}

impl<'tcx> TypeFolder<'tcx> for RemapLateBound<'_, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
if let ty::ReFree(fr) = *r {
self.tcx.mk_region(ty::ReFree(ty::FreeRegion {
bound_region: self
.mapping
.get(&fr.bound_region)
.copied()
.unwrap_or(fr.bound_region),
..fr
}))
} else {
r
}
}
}

fn emit_implied_wf_lint<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m: &ty::AssocItem,
hir_id: hir::HirId,
bad_args: Vec<(Span, Option<String>)>,
) {
let span: MultiSpan = if bad_args.is_empty() {
tcx.def_span(impl_m.def_id).into()
} else {
bad_args.iter().map(|(span, _)| *span).collect::<Vec<_>>().into()
};
tcx.struct_span_lint_hir(
rustc_session::lint::builtin::IMPLIED_BOUNDS_ENTAILMENT,
hir_id,
span,
"impl method assumes more implied bounds than the corresponding trait method",
|lint| {
let bad_args: Vec<_> =
bad_args.into_iter().filter_map(|(span, sugg)| Some((span, sugg?))).collect();
if !bad_args.is_empty() {
lint.multipart_suggestion(
format!(
"replace {} type{} to make the impl signature compatible",
pluralize!("this", bad_args.len()),
pluralize!(bad_args.len())
),
bad_args,
Applicability::MaybeIncorrect,
);
}
lint
},
);
}

#[derive(Debug, PartialEq, Eq)]
enum CheckImpliedWfMode {
/// Checks implied well-formedness of the impl method. If it fails, we will
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: impl method assumes more implied bounds than the corresponding trait method
--> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5
--> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:31
|
LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `()`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
Expand All @@ -16,10 +16,10 @@ error: aborting due to previous error

Future incompatibility report: Future breakage diagnostic:
error: impl method assumes more implied bounds than the corresponding trait method
--> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5
--> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:31
|
LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `()`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: impl method assumes more implied bounds than the corresponding trait method
--> $DIR/impl-implied-bounds-compatibility.rs:14:5
--> $DIR/impl-implied-bounds-compatibility.rs:14:35
|
LL | fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `&'b MessageListeners<'b>`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
Expand All @@ -16,10 +16,10 @@ error: aborting due to previous error

Future incompatibility report: Future breakage diagnostic:
error: impl method assumes more implied bounds than the corresponding trait method
--> $DIR/impl-implied-bounds-compatibility.rs:14:5
--> $DIR/impl-implied-bounds-compatibility.rs:14:35
|
LL | fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `&'b MessageListeners<'b>`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #105572 <https://github.com/rust-lang/rust/issues/105572>
Expand Down

0 comments on commit eaa7cc8

Please sign in to comment.