Auto merge of #117984 - compiler-errors:implied-bounds-entailment, r=<try>

Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint

closes #105572

Removes the `IMPLIED_BOUNDS_ENTAILMENT` and makes the `compare_method_predicate_entailment` logic just run once.

r? lcnr

Author: bors

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -2,9 +2,7 @@ use super::potentially_plural_count;
 use crate::errors::LifetimesOrBoundsMismatchOnTrait;
 use hir::def_id::{DefId, LocalDefId};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
-use rustc_errors::{
-    pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed, MultiSpan,
-};
+use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::intravisit;
@@ -50,13 +48,7 @@ pub(super) fn compare_impl_method<'tcx>(
 
     let _: Result<_, ErrorGuaranteed> = try {
         check_method_is_structurally_compatible(tcx, impl_m, trait_m, impl_trait_ref, false)?;
-        compare_method_predicate_entailment(
-            tcx,
-            impl_m,
-            trait_m,
-            impl_trait_ref,
-            CheckImpliedWfMode::Check,
-        )?;
+        compare_method_predicate_entailment(tcx, impl_m, trait_m, impl_trait_ref)?;
         refine::check_refining_return_position_impl_trait_in_trait(
             tcx,
             impl_m,
@@ -170,7 +162,6 @@ fn compare_method_predicate_entailment<'tcx>(
     impl_m: ty::AssocItem,
     trait_m: ty::AssocItem,
     impl_trait_ref: ty::TraitRef<'tcx>,
-    check_implied_wf: CheckImpliedWfMode,
 ) -> Result<(), ErrorGuaranteed> {
     let trait_to_impl_args = impl_trait_ref.args;
 
@@ -317,7 +308,7 @@ fn compare_method_predicate_entailment<'tcx>(
         return Err(emitted);
     }
 
-    if check_implied_wf == CheckImpliedWfMode::Check && !(impl_sig, trait_sig).references_error() {
+    if !(impl_sig, trait_sig).references_error() {
         // Select obligations to make progress on inference before processing
         // the wf obligation below.
         // FIXME(-Ztrait-solver=next): Not needed when the hack below is removed.
@@ -334,7 +325,8 @@ fn compare_method_predicate_entailment<'tcx>(
         // region outlives obligations.
         //
         // FIXME(-Ztrait-solver=next): Try removing this hack again once
-        // the new solver is stable.
+        // the new solver is stable. We should just be able to register a WF pred for
+        // the fn sig.
         let mut wf_args: smallvec::SmallVec<[_; 4]> =
             unnormalized_impl_sig.inputs_and_output.iter().map(|ty| ty.into()).collect();
         // Annoyingly, asking for the WF predicates of an array (with an unevaluated const (only?))
@@ -357,7 +349,7 @@ fn compare_method_predicate_entailment<'tcx>(
                     // We need to register Projection oblgiations too, because we may end up with
                     // an implied `X::Item: 'a`, which gets desugared into `X::Item = ?0`, `?0: 'a`.
                     // If we only register the region outlives obligation, this leads to an unconstrained var.
-                    // See `implied_bounds_entailment_alias_var` test.
+                    // See `implied_bounds_entailment_alias_var.rs` test.
                     ty::PredicateKind::Clause(
                         ty::ClauseKind::RegionOutlives(..)
                         | ty::ClauseKind::TypeOutlives(..)
@@ -378,26 +370,8 @@ fn compare_method_predicate_entailment<'tcx>(
     // version.
     let errors = ocx.select_all_or_error();
     if !errors.is_empty() {
-        match check_implied_wf {
-            CheckImpliedWfMode::Check => {
-                let impl_m_hir_id = tcx.hir().local_def_id_to_hir_id(impl_m_def_id);
-                return compare_method_predicate_entailment(
-                    tcx,
-                    impl_m,
-                    trait_m,
-                    impl_trait_ref,
-                    CheckImpliedWfMode::Skip,
-                )
-                .map(|()| {
-                    // If the skip-mode was successful, emit a lint.
-                    emit_implied_wf_lint(infcx.tcx, impl_m, impl_m_hir_id, vec![]);
-                });
-            }
-            CheckImpliedWfMode::Skip => {
-                let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
-                return Err(reported);
-            }
-        }
+        let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
+        return Err(reported);
     }
 
     // Finally, resolve all regions. This catches wily misuses of
@@ -408,119 +382,14 @@ fn compare_method_predicate_entailment<'tcx>(
     );
     let errors = infcx.resolve_regions(&outlives_env);
     if !errors.is_empty() {
-        // FIXME(compiler-errors): This can be simplified when IMPLIED_BOUNDS_ENTAILMENT
-        // becomes a hard error (i.e. ideally we'd just call `resolve_regions_and_report_errors`
-        let impl_m_hir_id = tcx.hir().local_def_id_to_hir_id(impl_m_def_id);
-        match check_implied_wf {
-            CheckImpliedWfMode::Check => {
-                return compare_method_predicate_entailment(
-                    tcx,
-                    impl_m,
-                    trait_m,
-                    impl_trait_ref,
-                    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(tcx, impl_m, impl_m_hir_id, bad_args);
-                });
-            }
-            CheckImpliedWfMode::Skip => {
-                if infcx.tainted_by_errors().is_none() {
-                    infcx.err_ctxt().report_region_errors(impl_m_def_id, &errors);
-                }
-                return Err(tcx
-                    .sess
-                    .delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted"));
-            }
-        }
+        return Err(infcx
+            .tainted_by_errors()
+            .unwrap_or_else(|| infcx.err_ctxt().report_region_errors(impl_m_def_id, &errors)));
     }
 
     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).skip_binder().bound_vars(),
-        tcx.fn_sig(impl_m.def_id).skip_binder().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>,
@@ -544,53 +413,6 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for RemapLateBound<'_, 'tcx> {
     }
 }
 
-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
-    /// re-check with `Skip`, and emit a lint if it succeeds.
-    Check,
-    /// Skips checking implied well-formedness of the impl method, but will emit
-    /// a lint if the `compare_method_predicate_entailment` succeeded. This means that
-    /// the reason that we had failed earlier during `Check` was due to the impl
-    /// having stronger requirements than the trait.
-    Skip,
-}
-
 fn compare_asyncness<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_m: ty::AssocItem,

compiler/rustc_lint_defs/src/builtin.rs

@@ -3387,7 +3387,6 @@ declare_lint_pass! {
         HIDDEN_GLOB_REEXPORTS,
         ILL_FORMED_ATTRIBUTE_INPUT,
         ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
-        IMPLIED_BOUNDS_ENTAILMENT,
         INCOMPLETE_INCLUDE,
         INDIRECT_STRUCTURAL_MATCH,
         INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
@@ -4232,47 +4231,6 @@ declare_lint! {
     "named arguments in format used positionally"
 }
 
-declare_lint! {
-    /// The `implied_bounds_entailment` lint detects cases where the arguments of an impl method
-    /// have stronger implied bounds than those from the trait method it's implementing.
-    ///
-    /// ### Example
-    ///
-    /// ```rust,compile_fail
-    /// #![deny(implied_bounds_entailment)]
-    ///
-    /// trait Trait {
-    ///     fn get<'s>(s: &'s str, _: &'static &'static ()) -> &'static str;
-    /// }
-    ///
-    /// impl Trait for () {
-    ///     fn get<'s>(s: &'s str, _: &'static &'s ()) -> &'static str {
-    ///         s
-    ///     }
-    /// }
-    ///
-    /// let val = <() as Trait>::get(&String::from("blah blah blah"), &&());
-    /// println!("{}", val);
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// Neither the trait method, which provides no implied bounds about `'s`, nor the impl,
-    /// requires the main function to prove that 's: 'static, but the impl method is allowed
-    /// to assume that `'s: 'static` within its own body.
-    ///
-    /// This can be used to implement an unsound API if used incorrectly.
-    pub IMPLIED_BOUNDS_ENTAILMENT,
-    Deny,
-    "impl method assumes more implied bounds than its corresponding trait method",
-    @future_incompatible = FutureIncompatibleInfo {
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
-        reference: "issue #105572 <https://github.com/rust-lang/rust/issues/105572>",
-    };
-}
-
 declare_lint! {
     /// The `byte_slice_in_packed_struct_with_derive` lint detects cases where a byte slice field
     /// (`[u8]`) or string slice field (`str`) is used in a `packed` struct that derives one or

tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.rs

@@ -1,5 +1,3 @@
-#![deny(implied_bounds_entailment)]
-
 trait Project {
     type Ty;
 }
@@ -11,8 +9,7 @@ trait Trait {
 }
 impl Trait for () {
     fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
-        //~^ ERROR impl method assumes more implied bounds than the corresponding trait method
-        //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+        //~^ ERROR cannot infer an appropriate lifetime for lifetime parameter 's in generic type due to conflicting requirements
         s
     }
 }

tests/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr

@@ -1,31 +1,28 @@
-error: impl method assumes more implied bounds than the corresponding trait method
-  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:31
+error[E0495]: cannot infer an appropriate lifetime for lifetime parameter 's in generic type due to conflicting requirements
+  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:11:5
    |
 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)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
-
-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
+note: first, the lifetime cannot outlive the lifetime `'s` as defined here...
+  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:11:12
    |
 LL |     fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
-   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this type to make the impl signature compatible: `()`
+   |            ^^
+note: ...so that the method type is compatible with trait
+  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:11:5
    |
-   = 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 |     fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: expected `fn(&'s str, ()) -> &'static str`
+              found `fn(&str, ()) -> &'static str`
+   = note: but, the lifetime must be valid for the static lifetime...
+note: ...so that the reference type `&'static &()` does not outlive the data it points at
+  --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:11:5
    |
-LL | #![deny(implied_bounds_entailment)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0495`.