Rollup merge of #106465 - compiler-errors:bump-IMPLIED_BOUNDS_ENTAILMENT, r=lcnr

matthiaskrgr · web-flow · commit 1dc43b2e8b5f · 2023-01-13T19:16:41.000+01:00
Bump `IMPLIED_BOUNDS_ENTAILMENT` to Deny + ReportNow #105575 (comment) > and then later in the same cycle increase the lint to `deny` and change it to `FutureCompatReportNow` in this nightly cycle. r? ```@lcnr``` when they're back from holiday 😄
diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
@@ -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;
@@ -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.
@@ -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 => {
@@ -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 => {
@@ -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
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -4033,10 +4033,10 @@ declare_lint! {
     ///
     /// This can be used to implement an unsound API if used incorrectly.
     pub IMPLIED_BOUNDS_ENTAILMENT,
-    Warn,
+    Deny,
     "impl method assumes more implied bounds than its corresponding trait method",
     @future_incompatible = FutureIncompatibleInfo {
         reference: "issue #105572 <https://github.com/rust-lang/rust/issues/105572>",
-        reason: FutureIncompatibilityReason::FutureReleaseError,
+        reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow,
     };
 }
diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr
@@ -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>
@@ -14,3 +14,18 @@ LL | #![deny(implied_bounds_entailment)]
 
 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: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>
+note: the lint level is defined here
+  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:1:9
+   |
+LL | #![deny(implied_bounds_entailment)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+
diff --git a/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr b/tests/ui/implied-bounds/impl-implied-bounds-compatibility.stderr
@@ -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>
@@ -14,3 +14,18 @@ LL | #![deny(implied_bounds_entailment)]
 
 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: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>
+note: the lint level is defined here
+  --> $DIR/impl-implied-bounds-compatibility.rs:1:9
+   |
+LL | #![deny(implied_bounds_entailment)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+
