diff options
9 files changed, 223 insertions, 15 deletions
diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index b69728c24aa..cddd307c13d 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -71,8 +71,14 @@ pub(crate) fn compare_impl_method<'tcx>( return; } - if let Err(_) = compare_predicate_entailment(tcx, impl_m, impl_m_span, trait_m, impl_trait_ref) - { + if let Err(_) = compare_predicate_entailment( + tcx, + impl_m, + impl_m_span, + trait_m, + impl_trait_ref, + CheckImpliedWfMode::Check, + ) { return; } } @@ -150,6 +156,7 @@ fn compare_predicate_entailment<'tcx>( impl_m_span: Span, trait_m: &ty::AssocItem, impl_trait_ref: ty::TraitRef<'tcx>, + check_implied_wf: CheckImpliedWfMode, ) -> Result<(), ErrorGuaranteed> { let trait_to_impl_substs = impl_trait_ref.substs; @@ -255,15 +262,15 @@ fn compare_predicate_entailment<'tcx>( let mut wf_tys = FxIndexSet::default(); - let impl_sig = infcx.replace_bound_vars_with_fresh_vars( + let unnormalized_impl_sig = infcx.replace_bound_vars_with_fresh_vars( impl_m_span, infer::HigherRankedType, tcx.fn_sig(impl_m.def_id), ); + let unnormalized_impl_fty = tcx.mk_fn_ptr(ty::Binder::dummy(unnormalized_impl_sig)); let norm_cause = ObligationCause::misc(impl_m_span, impl_m_hir_id); - let impl_sig = ocx.normalize(&norm_cause, param_env, impl_sig); - let impl_fty = tcx.mk_fn_ptr(ty::Binder::dummy(impl_sig)); + let impl_fty = ocx.normalize(&norm_cause, param_env, unnormalized_impl_fty); debug!("compare_impl_method: impl_fty={:?}", impl_fty); let trait_sig = tcx.bound_fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs); @@ -304,29 +311,108 @@ fn compare_predicate_entailment<'tcx>( return Err(emitted); } + if check_implied_wf == CheckImpliedWfMode::Check { + // We need to check that the impl's args are well-formed given + // the hybrid param-env (impl + trait method where-clauses). + ocx.register_obligation(traits::Obligation::new( + infcx.tcx, + ObligationCause::dummy(), + param_env, + 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. let errors = ocx.select_all_or_error(); if !errors.is_empty() { - let reported = infcx.err_ctxt().report_fulfillment_errors(&errors, None); - return Err(reported); + match check_implied_wf { + CheckImpliedWfMode::Check => { + return compare_predicate_entailment( + tcx, + impl_m, + impl_m_span, + trait_m, + impl_trait_ref, + CheckImpliedWfMode::Skip, + ) + .map(|()| { + // If the skip-mode was successful, emit a lint. + emit_implied_wf_lint(); + }); + } + CheckImpliedWfMode::Skip => { + let reported = infcx.err_ctxt().report_fulfillment_errors(&errors, None); + return Err(reported); + } + } } // Finally, resolve all regions. This catches wily misuses of // lifetime parameters. - let outlives_environment = OutlivesEnvironment::with_bounds( + let outlives_env = OutlivesEnvironment::with_bounds( param_env, Some(infcx), - infcx.implied_bounds_tys(param_env, impl_m_hir_id, wf_tys), + infcx.implied_bounds_tys(param_env, impl_m_hir_id, wf_tys.clone()), ); - infcx.check_region_obligations_and_report_errors( - impl_m.def_id.expect_local(), - &outlives_environment, + infcx.process_registered_region_obligations( + outlives_env.region_bound_pairs(), + outlives_env.param_env, ); + 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` + match check_implied_wf { + CheckImpliedWfMode::Check => { + return compare_predicate_entailment( + tcx, + impl_m, + impl_m_span, + trait_m, + impl_trait_ref, + CheckImpliedWfMode::Skip, + ) + .map(|()| { + // If the skip-mode was successful, emit a lint. + emit_implied_wf_lint(); + }); + } + CheckImpliedWfMode::Skip => { + if infcx.tainted_by_errors().is_none() { + infcx.err_ctxt().report_region_errors(impl_m.def_id.expect_local(), &errors); + } + return Err(tcx + .sess + .delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted")); + } + } + } Ok(()) } +#[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_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, diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 268b3bf1dcd..a9ef91db059 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -1693,7 +1693,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { &self, generic_param_scope: LocalDefId, outlives_env: &OutlivesEnvironment<'tcx>, - ) { + ) -> Option<ErrorGuaranteed> { let errors = self.resolve_regions(outlives_env); if let None = self.tainted_by_errors() { @@ -1704,6 +1704,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // errors from silly ones. self.report_region_errors(generic_param_scope, &errors); } + + (!errors.is_empty()).then(|| { + self.tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted") + }) } // [Note-Type-error-reporting] diff --git a/compiler/rustc_infer/src/infer/outlives/obligations.rs b/compiler/rustc_infer/src/infer/outlives/obligations.rs index ccae7165d80..47bd1564f08 100644 --- a/compiler/rustc_infer/src/infer/outlives/obligations.rs +++ b/compiler/rustc_infer/src/infer/outlives/obligations.rs @@ -68,6 +68,7 @@ use crate::infer::{ }; use crate::traits::{ObligationCause, ObligationCauseCode}; use rustc_data_structures::undo_log::UndoLogs; +use rustc_errors::ErrorGuaranteed; use rustc_hir::def_id::DefId; use rustc_hir::def_id::LocalDefId; use rustc_middle::mir::ConstraintCategory; @@ -177,7 +178,7 @@ impl<'tcx> InferCtxt<'tcx> { &self, generic_param_scope: LocalDefId, outlives_env: &OutlivesEnvironment<'tcx>, - ) { + ) -> Option<ErrorGuaranteed> { self.process_registered_region_obligations( outlives_env.region_bound_pairs(), outlives_env.param_env, diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 33cb35e60eb..f7a4103f4d5 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3311,6 +3311,7 @@ declare_lint_pass! { FFI_UNWIND_CALLS, REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS, NAMED_ARGUMENTS_USED_POSITIONALLY, + IMPLIED_BOUNDS_ENTAILMENT, ] } @@ -3998,3 +3999,44 @@ declare_lint! { Warn, "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, + Warn, + "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, + }; +} diff --git a/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.rs b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.rs new file mode 100644 index 00000000000..6ccbb5bb266 --- /dev/null +++ b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.rs @@ -0,0 +1,22 @@ +#![deny(implied_bounds_entailment)] + +trait Project { + type Ty; +} +impl Project for &'_ &'_ () { + type Ty = (); +} +trait Trait { + fn get<'s>(s: &'s str, _: ()) -> &'static str; +} +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! + s + } +} +fn main() { + let val = <() as Trait>::get(&String::from("blah blah blah"), ()); + println!("{}", val); +} diff --git a/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr new file mode 100644 index 00000000000..0ac31c642eb --- /dev/null +++ b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility-unnormalized.stderr @@ -0,0 +1,16 @@ +error: impl method assumes more implied bounds than the corresponding trait method + --> $DIR/impl-implied-bounds-compatibility-unnormalized.rs:13:5 + | +LL | fn get<'s>(s: &'s str, _: <&'static &'s () as Project>::Ty) -> &'static str { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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 + diff --git a/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.rs b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.rs new file mode 100644 index 00000000000..d097bc16a22 --- /dev/null +++ b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.rs @@ -0,0 +1,21 @@ +#![deny(implied_bounds_entailment)] + +use std::cell::RefCell; + +pub struct MessageListeners<'a> { + listeners: RefCell<Vec<Box<dyn FnMut(()) + 'a>>>, +} + +pub trait MessageListenersInterface { + fn listeners<'c>(&'c self) -> &'c MessageListeners<'c>; +} + +impl<'a> MessageListenersInterface for MessageListeners<'a> { + fn listeners<'b>(&'b self) -> &'a MessageListeners<'b> { + //~^ 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! + self + } +} + +fn main() {} diff --git a/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.stderr b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.stderr new file mode 100644 index 00000000000..0dfa8167a99 --- /dev/null +++ b/src/test/ui/implied-bounds/impl-implied-bounds-compatibility.stderr @@ -0,0 +1,16 @@ +error: impl method assumes more implied bounds than the corresponding trait method + --> $DIR/impl-implied-bounds-compatibility.rs:14:5 + | +LL | fn listeners<'b>(&'b self) -> &'a 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)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/tools/clippy/tests/ui/borrow_interior_mutable_const/others.rs b/src/tools/clippy/tests/ui/borrow_interior_mutable_const/others.rs index eefeb1decb6..7c57864245a 100644 --- a/src/tools/clippy/tests/ui/borrow_interior_mutable_const/others.rs +++ b/src/tools/clippy/tests/ui/borrow_interior_mutable_const/others.rs @@ -42,7 +42,7 @@ impl<T> StaticRef<T> { impl<T> std::ops::Deref for StaticRef<T> { type Target = T; - fn deref(&self) -> &'static T { + fn deref(&self) -> &T { unsafe { &*self.ptr } } } |