Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IMPLIED_BOUNDS_ENTAILMENT lint #105575

Merged
merged 4 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 98 additions & 12 deletions compiler/rustc_hir_analysis/src/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3311,6 +3311,7 @@ declare_lint_pass! {
FFI_UNWIND_CALLS,
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
NAMED_ARGUMENTS_USED_POSITIONALLY,
IMPLIED_BOUNDS_ENTAILMENT,
]
}

Expand Down Expand Up @@ -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,
};
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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

21 changes: 21 additions & 0 deletions src/test/ui/implied-bounds/impl-implied-bounds-compatibility.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
Expand Down