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

lint incorrect implied bounds in wfcheck #109763

Closed
wants to merge 3 commits into from
Closed
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
7 changes: 4 additions & 3 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ fn compare_method_predicate_entailment<'tcx>(
// lifetime parameters.
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()),
infcx.implied_bounds_tys_compat(param_env, impl_m_def_id, wf_tys.clone()),
);
let errors = infcx.resolve_regions(&outlives_env);
if !errors.is_empty() {
Expand Down Expand Up @@ -724,7 +724,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
// lifetime parameters.
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys),
infcx.implied_bounds_tys_compat(param_env, impl_m_def_id, wf_tys),
);
ocx.resolve_regions_and_report_errors(impl_m_def_id, &outlives_env)?;

Expand Down Expand Up @@ -2050,7 +2050,8 @@ pub(super) fn check_type_bounds<'tcx>(

// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let implied_bounds = infcx.implied_bounds_tys(param_env, impl_ty_def_id, assumed_wf_types);
let implied_bounds =
infcx.implied_bounds_tys_compat(param_env, impl_ty_def_id, assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
ocx.resolve_regions_and_report_errors(impl_ty_def_id, &outlives_env)
}
Expand Down
41 changes: 38 additions & 3 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,52 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
f(&mut wfcx);

let assumed_wf_types = wfcx.ocx.assumed_wf_types(param_env, span, body_def_id);
let implied_bounds = infcx.implied_bounds_tys(param_env, body_def_id, assumed_wf_types);

let errors = wfcx.select_all_or_error();
if !errors.is_empty() {
infcx.err_ctxt().report_fulfillment_errors(&errors);
return;
}

let infcx_compat = infcx.fork();

let implied_bounds = infcx.implied_bounds_tys(param_env, &assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let errors = infcx.resolve_regions(&outlives_env);
if errors.is_empty() {
return;
}

let implied_bounds =
infcx_compat.implied_bounds_tys_compat(param_env, body_def_id, assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let errors_compat = infcx_compat.resolve_regions(&outlives_env);
if !errors_compat.is_empty() {
infcx_compat.err_ctxt().report_region_errors(body_def_id, &errors_compat);
return;
}

let _ = wfcx.ocx.resolve_regions_and_report_errors(body_def_id, &outlives_env);
let hir_id = tcx.hir().local_def_id_to_hir_id(body_def_id);
let (lint_level, _) = tcx
.lint_level_at_node(rustc_session::lint::builtin::IMPLIED_BOUNDS_FROM_TRAIT_IMPL, hir_id);
tcx.struct_span_lint_hir(
rustc_session::lint::builtin::IMPLIED_BOUNDS_FROM_TRAIT_IMPL,
hir_id,
tcx.def_span(body_def_id),
format!("{} is missing necessary lifetime bounds", tcx.def_descr(body_def_id.into())),
|lint| {
if !lint_level.is_error() {
lint.note(
"to get more detailed errors, use `#[deny(implied_bounds_from_trait_impl)]`",
)
} else {
lint.note("more concrete lifetime errors are emitted below")
}
},
);
if lint_level.is_error() {
infcx.err_ctxt().report_region_errors(body_def_id, &errors);
}
}

fn check_well_formed(tcx: TyCtxt<'_>, def_id: hir::OwnerId) {
Expand Down Expand Up @@ -674,7 +709,7 @@ fn resolve_regions_with_wf_tys<'tcx>(
let infcx = tcx.infer_ctxt().build();
let outlives_environment = OutlivesEnvironment::with_bounds(
param_env,
infcx.implied_bounds_tys(param_env, id, wf_tys.clone()),
infcx.implied_bounds_tys_compat(param_env, id, wf_tys.clone()),
);
let region_bound_pairs = outlives_environment.region_bound_pairs();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ fn get_impl_substs(
return None;
}

let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types);
let implied_bounds = infcx.implied_bounds_tys_compat(param_env, impl1_def_id, assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let _ = ocx.resolve_regions_and_report_errors(impl1_def_id, &outlives_env);
let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
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 @@ -3304,6 +3304,7 @@ declare_lint_pass! {
ILL_FORMED_ATTRIBUTE_INPUT,
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
IMPLIED_BOUNDS_ENTAILMENT,
IMPLIED_BOUNDS_FROM_TRAIT_IMPL,
INCOMPLETE_INCLUDE,
INDIRECT_STRUCTURAL_MATCH,
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
Expand Down Expand Up @@ -4172,3 +4173,44 @@ declare_lint! {
Warn,
"\"invalid_parameter\" isn't a valid argument for `#[macro_export]`",
}

declare_lint! {
/// The `implied_bounds_from_trait_impl` lint detects
/// a compiler bug allowed some code to assume invalid implied lifetime bounds.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(implied_bounds_from_trait_impl)]
///
/// trait Trait {
/// type Assoc;
/// }
///
/// impl<X: 'static> Trait for (X,) {
/// type Assoc = ();
/// }
///
/// struct Foo<T: Trait>(T)
/// where
/// T::Assoc: Clone; // any bound using `T::Assoc`
///
/// fn func(foo: Foo<(&str,)>) {
/// let _: &'static str = foo.0.0;
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// FIXME: explanataion
///
pub IMPLIED_BOUNDS_FROM_TRAIT_IMPL,
Warn,
"item uses illegal implied bounds derived from a trait impl",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #109628 <https://github.com/rust-lang/rust/issues/109628>",
reason: FutureIncompatibilityReason::FutureReleaseError,
};
}
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ macro_rules! arena_types {
rustc_middle::traits::query::NormalizationResult<'tcx>
>
>,
[] implied_outlives_bounds:
[] implied_outlives_bounds_compat:
rustc_middle::infer::canonical::Canonical<'tcx,
rustc_middle::infer::canonical::QueryResponse<'tcx,
Vec<rustc_middle::traits::query::OutlivesBound<'tcx>>
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ impl<T> EraseType for Result<&'_ T, traits::query::NoSolution> {
type Result = [u8; size_of::<Result<&'static (), traits::query::NoSolution>>()];
}

impl<T> EraseType for Result<&'_ [T], traits::query::NoSolution> {
type Result = [u8; size_of::<Result<&'static [()], traits::query::NoSolution>>()];
}

impl<T> EraseType for Result<&'_ T, rustc_errors::ErrorGuaranteed> {
type Result = [u8; size_of::<Result<&'static (), rustc_errors::ErrorGuaranteed>>()];
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1828,7 +1828,7 @@ rustc_queries! {
desc { "normalizing `{}`", goal.value }
}

query implied_outlives_bounds(
query implied_outlives_bounds_compat(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that it seems like most places still use this, it seems like it would make sense to keep this name the same, and name the new version v2 or something.

goal: CanonicalTyGoal<'tcx>
) -> Result<
&'tcx Canonical<'tcx, canonical::QueryResponse<'tcx, Vec<OutlivesBound<'tcx>>>>,
Expand All @@ -1837,6 +1837,12 @@ rustc_queries! {
desc { "computing implied outlives bounds for `{}`", goal.value.value }
}

query implied_outlives_bounds(
goal: ParamEnvAnd<'tcx, Ty<'tcx>>
) -> Result<&'tcx [OutlivesBound<'tcx>], NoSolution> {
desc { "computing implied outlives bounds v2 for `{}`", goal.value }
}

/// Do not call this query directly:
/// invoke `DropckOutlives::new(dropped_ty)).fully_perform(typeck.infcx)` instead.
query dropck_outlives(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub struct NormalizationResult<'tcx> {
/// case they are called implied bounds). They are fed to the
/// `OutlivesEnv` which in turn is supplied to the region checker and
/// other parts of the inference system.
#[derive(Clone, Debug, TypeFoldable, TypeVisitable, Lift, HashStable)]
#[derive(Copy, Clone, Debug, TypeFoldable, TypeVisitable, Lift, HashStable)]
pub enum OutlivesBound<'tcx> {
RegionSubRegion(ty::Region<'tcx>, ty::Region<'tcx>),
RegionSubParam(ty::Region<'tcx>, ty::ParamTy),
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
arg: ty::GenericArg<'tcx>,
) -> Option<impl Iterator<Item = Goal<'tcx, ty::Predicate<'tcx>>>> {
crate::traits::wf::unnormalized_obligations(self.infcx, param_env, arg)
.map(|obligations| obligations.into_iter().map(|obligation| obligation.into()))
debug_assert_eq!(arg, self.infcx.resolve_vars_if_possible(arg));
// FIXME: useless `Option` in return type?
Some(
crate::traits::wf::unnormalized_obligations(self.tcx(), param_env, arg)
.into_iter()
.map(|obligation| obligation.into()),
)
}

pub(super) fn is_transmutable(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ fn resolve_negative_obligation<'tcx>(
let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id);
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
infcx.implied_bounds_tys(param_env, body_def_id, wf_tys),
infcx.implied_bounds_tys_compat(param_env, body_def_id, wf_tys),
);
infcx.resolve_regions(&outlives_env).is_empty()
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ pub fn all_fields_implement_trait<'tcx>(
// Check regions assuming the self type of the impl is WF
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
infcx.implied_bounds_tys(
infcx.implied_bounds_tys_compat(
param_env,
parent_cause.body_id,
FxIndexSet::from_iter([self_type]),
Expand Down
41 changes: 35 additions & 6 deletions compiler/rustc_trait_selection/src/traits/outlives_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,34 @@ use crate::traits::query::type_op::{self, TypeOp, TypeOpOutput};
use crate::traits::query::NoSolution;
use crate::traits::{ObligationCause, ObligationCtxt};
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::def_id::LocalDefId;
use rustc_infer::infer::resolve::OpportunisticRegionResolver;
use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt};
use rustc_span::def_id::LocalDefId;
use rustc_span::DUMMY_SP;

pub use rustc_middle::traits::query::OutlivesBound;

type BoundsCompat<'a, 'tcx: 'a> = impl Iterator<Item = OutlivesBound<'tcx>> + 'a;
type Bounds<'a, 'tcx: 'a> = impl Iterator<Item = OutlivesBound<'tcx>> + 'a;
pub trait InferCtxtExt<'a, 'tcx> {
fn implied_outlives_bounds(
fn implied_outlives_bounds_compat(
&self,
param_env: ty::ParamEnv<'tcx>,
body_id: LocalDefId,
ty: Ty<'tcx>,
) -> Vec<OutlivesBound<'tcx>>;

fn implied_bounds_tys(
fn implied_bounds_tys_compat(
&'a self,
param_env: ty::ParamEnv<'tcx>,
body_id: LocalDefId,
tys: FxIndexSet<Ty<'tcx>>,
) -> BoundsCompat<'a, 'tcx>;

fn implied_bounds_tys(
&'a self,
param_env: ty::ParamEnv<'tcx>,
tys: &'a FxIndexSet<Ty<'tcx>>,
) -> Bounds<'a, 'tcx>;
}

Expand All @@ -47,7 +55,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
/// into the inference context with this body-id.
/// - `ty`, the type that we are supposed to assume is WF.
#[instrument(level = "debug", skip(self, param_env, body_id), ret)]
fn implied_outlives_bounds(
fn implied_outlives_bounds_compat(
&self,
param_env: ty::ParamEnv<'tcx>,
body_id: LocalDefId,
Expand Down Expand Up @@ -115,12 +123,33 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
output
}

fn implied_bounds_tys(
fn implied_bounds_tys_compat(
&'a self,
param_env: ParamEnv<'tcx>,
body_id: LocalDefId,
tys: FxIndexSet<Ty<'tcx>>,
) -> BoundsCompat<'a, 'tcx> {
tys.into_iter()
.flat_map(move |ty| self.implied_outlives_bounds_compat(param_env, body_id, ty))
}

fn implied_bounds_tys(
&'a self,
param_env: ParamEnv<'tcx>,
tys: &'a FxIndexSet<Ty<'tcx>>,
) -> Bounds<'a, 'tcx> {
tys.into_iter().flat_map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty))
tys.iter()
.flat_map(move |&ty| {
let ty = self.resolve_vars_if_possible(ty);
let ty = OpportunisticRegionResolver::new(self).fold_ty(ty);
if ty.has_infer() {
// Infer vars can appear only in invalid code. See #110161.
self.tcx.sess.delay_span_bug(DUMMY_SP, "infer vars in implied bounds");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this line should be removed when/if this is rebased #115559

return &[] as &[OutlivesBound<'_>];
}

self.tcx.implied_outlives_bounds(param_env.and(ty)).unwrap_or(&[])
})
.copied()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ impl<'tcx> super::QueryTypeOp<'tcx> for ImpliedOutlivesBounds<'tcx> {
param_env.and(ty)
});

tcx.implied_outlives_bounds(canonicalized)
tcx.implied_outlives_bounds_compat(canonicalized)
}
}
17 changes: 8 additions & 9 deletions compiler/rustc_trait_selection/src/traits/wf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,20 @@ pub fn obligations<'tcx>(

/// Compute the predicates that are required for a type to be well-formed.
///
/// This is only intended to be used in the new solver, since it does not
/// take into account recursion depth or proper error-reporting spans.
/// This is only intended to be used in implied bounds computation and in
/// the new solver, since it does not take into account recursion depth or
/// proper error-reporting spans.
pub fn unnormalized_obligations<'tcx>(
infcx: &InferCtxt<'tcx>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
arg: GenericArg<'tcx>,
) -> Option<Vec<traits::PredicateObligation<'tcx>>> {
) -> Vec<traits::PredicateObligation<'tcx>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too can be moved into a separate commit or PR?

if let ty::GenericArgKind::Lifetime(..) = arg.unpack() {
return Some(vec![]);
return vec![];
}

debug_assert_eq!(arg, infcx.resolve_vars_if_possible(arg));

let mut wf = WfPredicates {
tcx: infcx.tcx,
tcx,
param_env,
body_id: CRATE_DEF_ID,
span: DUMMY_SP,
Expand All @@ -100,7 +99,7 @@ pub fn unnormalized_obligations<'tcx>(
item: None,
};
wf.compute(arg);
Some(wf.out)
wf.out
}

/// Returns the obligations that make this trait reference
Expand Down
Loading