Skip to content

Commit

Permalink
Auto merge of #100676 - lcnr:implied-bounds-yay, r=nikomatsakis
Browse files Browse the repository at this point in the history
implied bounds: explicitly state which types are assumed to be wf

Adds a new query which maps each definition to the types which that definition assumes to be well formed. The intent is to make it easier to reason about implied bounds.

This change should not influence the user-facing behavior of rustc. Notably, `borrowck` still only assumes that the function signature of associated functions is well formed while `wfcheck` assumes that the both the function signature and the impl trait ref is well formed. Not sure if that by itself can trigger UB or whether it's just annoying.

As a next step, we can add `WellFormed` predicates to `predicates_of` of these items and can stop adding the wf bounds at each place which uses them. I also intend to move the computation from `assumed_wf_types` to `implied_bounds` into the `param_env` computation. This requires me to take a deeper look at `compare_predicate_entailment` which is currently somewhat weird wrt implied bounds so I am not touching this here.

r? `@nikomatsakis`
  • Loading branch information
bors committed Aug 22, 2022
2 parents d0ea1d7 + 736288f commit a9bb589
Show file tree
Hide file tree
Showing 24 changed files with 321 additions and 237 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,14 @@ rustc_queries! {
desc { |tcx| "processing `{}`", tcx.def_path_str(key.to_def_id()) }
}

/// Returns the types assumed to be well formed while "inside" of the given item.
///
/// Note that we've liberated the late bound regions of function signatures, so
/// this can not be used to check whether these types are well formed.
query assumed_wf_types(key: DefId) -> &'tcx ty::List<Ty<'tcx>> {
desc { |tcx| "computing the implied bounds of {}", tcx.def_path_str(key) }
}

/// Computes the signature of the function.
query fn_sig(key: DefId) -> ty::PolyFnSig<'tcx> {
desc { |tcx| "computing function signature of `{}`", tcx.def_path_str(key) }
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ impl<T> List<T> {
pub fn len(&self) -> usize {
self.len
}

pub fn as_slice(&self) -> &[T] {
self
}
}

impl<T: Copy> List<T> {
Expand Down
23 changes: 22 additions & 1 deletion compiler/rustc_trait_selection/src/traits/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::cell::RefCell;
use super::TraitEngine;
use super::{ChalkFulfillmentContext, FulfillmentContext};
use crate::infer::InferCtxtExt;
use rustc_hir::def_id::DefId;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::{InferCtxt, InferOk};
use rustc_infer::traits::{
FulfillmentError, Obligation, ObligationCause, PredicateObligation, TraitEngineExt as _,
Expand All @@ -12,6 +13,7 @@ use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::ToPredicate;
use rustc_middle::ty::TypeFoldable;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::Span;

pub trait TraitEngineExt<'tcx> {
fn new(tcx: TyCtxt<'tcx>) -> Box<Self>;
Expand Down Expand Up @@ -109,4 +111,23 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> {
pub fn select_all_or_error(&self) -> Vec<FulfillmentError<'tcx>> {
self.engine.borrow_mut().select_all_or_error(self.infcx)
}

pub fn assumed_wf_types(
&self,
param_env: ty::ParamEnv<'tcx>,
span: Span,
def_id: LocalDefId,
) -> FxHashSet<Ty<'tcx>> {
let tcx = self.infcx.tcx;
let assumed_wf_types = tcx.assumed_wf_types(def_id);
let mut implied_bounds = FxHashSet::default();
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
let cause = ObligationCause::misc(span, hir_id);
for ty in assumed_wf_types {
implied_bounds.insert(ty);
let normalized = self.normalize(cause.clone(), param_env, ty);
implied_bounds.insert(normalized);
}
implied_bounds
}
}
60 changes: 60 additions & 0 deletions compiler/rustc_ty_utils/src/implied_bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use crate::rustc_middle::ty::DefIdTree;
use rustc_hir::{def::DefKind, def_id::DefId};
use rustc_middle::ty::{self, Ty, TyCtxt};

pub fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers { assumed_wf_types, ..*providers };
}

fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx ty::List<Ty<'tcx>> {
match tcx.def_kind(def_id) {
DefKind::Fn => {
let sig = tcx.fn_sig(def_id);
let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
liberated_sig.inputs_and_output
}
DefKind::AssocFn => {
let sig = tcx.fn_sig(def_id);
let liberated_sig = tcx.liberate_late_bound_regions(def_id, sig);
let mut assumed_wf_types: Vec<_> =
tcx.assumed_wf_types(tcx.parent(def_id)).as_slice().into();
assumed_wf_types.extend(liberated_sig.inputs_and_output);
tcx.intern_type_list(&assumed_wf_types)
}
DefKind::Impl => match tcx.impl_trait_ref(def_id) {
Some(trait_ref) => {
let types: Vec<_> = trait_ref.substs.types().collect();
tcx.intern_type_list(&types)
}
// Only the impl self type
None => tcx.intern_type_list(&[tcx.type_of(def_id)]),
},
DefKind::AssocConst | DefKind::AssocTy => tcx.assumed_wf_types(tcx.parent(def_id)),
DefKind::Mod
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Variant
| DefKind::Trait
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::TyParam
| DefKind::Const
| DefKind::ConstParam
| DefKind::Static(_)
| DefKind::Ctor(_, _)
| DefKind::Macro(_)
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure
| DefKind::Generator => ty::List::empty(),
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_ty_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use rustc_middle::ty::query::Providers;
mod assoc;
mod common_traits;
mod consts;
mod implied_bounds;
pub mod instance;
mod needs_drop;
pub mod representability;
Expand All @@ -30,6 +31,7 @@ pub fn provide(providers: &mut Providers) {
assoc::provide(providers);
common_traits::provide(providers);
consts::provide(providers);
implied_bounds::provide(providers);
needs_drop::provide(providers);
ty::provide(providers);
instance::provide(providers);
Expand Down
146 changes: 71 additions & 75 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::potentially_plural_count;
use crate::check::regionck::OutlivesEnvironmentExt;
use crate::check::wfcheck;
use crate::errors::LifetimesOrBoundsMismatchOnTrait;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticId, ErrorGuaranteed};
Expand Down Expand Up @@ -71,6 +70,72 @@ pub(crate) fn compare_impl_method<'tcx>(
}
}

/// This function is best explained by example. Consider a trait:
///
/// trait Trait<'t, T> {
/// // `trait_m`
/// fn method<'a, M>(t: &'t T, m: &'a M) -> Self;
/// }
///
/// And an impl:
///
/// impl<'i, 'j, U> Trait<'j, &'i U> for Foo {
/// // `impl_m`
/// fn method<'b, N>(t: &'j &'i U, m: &'b N) -> Foo;
/// }
///
/// We wish to decide if those two method types are compatible.
/// For this we have to show that, assuming the bounds of the impl hold, the
/// bounds of `trait_m` imply the bounds of `impl_m`.
///
/// We start out with `trait_to_impl_substs`, that maps the trait
/// type parameters to impl type parameters. This is taken from the
/// impl trait reference:
///
/// trait_to_impl_substs = {'t => 'j, T => &'i U, Self => Foo}
///
/// We create a mapping `dummy_substs` that maps from the impl type
/// parameters to fresh types and regions. For type parameters,
/// this is the identity transform, but we could as well use any
/// placeholder types. For regions, we convert from bound to free
/// regions (Note: but only early-bound regions, i.e., those
/// declared on the impl or used in type parameter bounds).
///
/// impl_to_placeholder_substs = {'i => 'i0, U => U0, N => N0 }
///
/// Now we can apply `placeholder_substs` to the type of the impl method
/// to yield a new function type in terms of our fresh, placeholder
/// types:
///
/// <'b> fn(t: &'i0 U0, m: &'b) -> Foo
///
/// We now want to extract and substitute the type of the *trait*
/// method and compare it. To do so, we must create a compound
/// substitution by combining `trait_to_impl_substs` and
/// `impl_to_placeholder_substs`, and also adding a mapping for the method
/// type parameters. We extend the mapping to also include
/// the method parameters.
///
/// trait_to_placeholder_substs = { T => &'i0 U0, Self => Foo, M => N0 }
///
/// Applying this to the trait method type yields:
///
/// <'a> fn(t: &'i0 U0, m: &'a) -> Foo
///
/// This type is also the same but the name of the bound region (`'a`
/// vs `'b`). However, the normal subtyping rules on fn types handle
/// this kind of equivalency just fine.
///
/// We now use these substitutions to ensure that all declared bounds are
/// satisfied by the implementation's method.
///
/// We do this by creating a parameter environment which contains a
/// substitution corresponding to `impl_to_placeholder_substs`. We then build
/// `trait_to_placeholder_substs` and use it to convert the predicates contained
/// in the `trait_m` generics to the placeholder form.
///
/// Finally we register each of these predicates as an obligation and check that
/// they hold.
fn compare_predicate_entailment<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m: &ty::AssocItem,
Expand All @@ -97,69 +162,6 @@ fn compare_predicate_entailment<'tcx>(
},
);

// This code is best explained by example. Consider a trait:
//
// trait Trait<'t, T> {
// fn method<'a, M>(t: &'t T, m: &'a M) -> Self;
// }
//
// And an impl:
//
// impl<'i, 'j, U> Trait<'j, &'i U> for Foo {
// fn method<'b, N>(t: &'j &'i U, m: &'b N) -> Foo;
// }
//
// We wish to decide if those two method types are compatible.
//
// We start out with trait_to_impl_substs, that maps the trait
// type parameters to impl type parameters. This is taken from the
// impl trait reference:
//
// trait_to_impl_substs = {'t => 'j, T => &'i U, Self => Foo}
//
// We create a mapping `dummy_substs` that maps from the impl type
// parameters to fresh types and regions. For type parameters,
// this is the identity transform, but we could as well use any
// placeholder types. For regions, we convert from bound to free
// regions (Note: but only early-bound regions, i.e., those
// declared on the impl or used in type parameter bounds).
//
// impl_to_placeholder_substs = {'i => 'i0, U => U0, N => N0 }
//
// Now we can apply placeholder_substs to the type of the impl method
// to yield a new function type in terms of our fresh, placeholder
// types:
//
// <'b> fn(t: &'i0 U0, m: &'b) -> Foo
//
// We now want to extract and substitute the type of the *trait*
// method and compare it. To do so, we must create a compound
// substitution by combining trait_to_impl_substs and
// impl_to_placeholder_substs, and also adding a mapping for the method
// type parameters. We extend the mapping to also include
// the method parameters.
//
// trait_to_placeholder_substs = { T => &'i0 U0, Self => Foo, M => N0 }
//
// Applying this to the trait method type yields:
//
// <'a> fn(t: &'i0 U0, m: &'a) -> Foo
//
// This type is also the same but the name of the bound region ('a
// vs 'b). However, the normal subtyping rules on fn types handle
// this kind of equivalency just fine.
//
// We now use these substitutions to ensure that all declared bounds are
// satisfied by the implementation's method.
//
// We do this by creating a parameter environment which contains a
// substitution corresponding to impl_to_placeholder_substs. We then build
// trait_to_placeholder_substs and use it to convert the predicates contained
// in the trait_m.generics to the placeholder form.
//
// Finally we register each of these predicates as an obligation in
// a fresh FulfillmentCtxt, and invoke select_all_or_error.

// Create mapping from impl to placeholder.
let impl_to_placeholder_substs = InternalSubsts::identity_for_item(tcx, impl_m.def_id);

Expand Down Expand Up @@ -1445,14 +1447,17 @@ pub fn check_type_bounds<'tcx>(
};
debug!(?normalize_param_env);

let impl_ty_hir_id = tcx.hir().local_def_id_to_hir_id(impl_ty.def_id.expect_local());
let impl_ty_substs = InternalSubsts::identity_for_item(tcx, impl_ty.def_id);
let rebased_substs = impl_ty_substs.rebase_onto(tcx, container_id, impl_trait_ref.substs);

tcx.infer_ctxt().enter(move |infcx| {
let ocx = ObligationCtxt::new(&infcx);

let assumed_wf_types =
ocx.assumed_wf_types(param_env, impl_ty_span, impl_ty.def_id.expect_local());

let mut selcx = traits::SelectionContext::new(&infcx);
let impl_ty_hir_id = tcx.hir().local_def_id_to_hir_id(impl_ty.def_id.expect_local());
let normalize_cause = ObligationCause::new(
impl_ty_span,
impl_ty_hir_id,
Expand Down Expand Up @@ -1508,17 +1513,8 @@ pub fn check_type_bounds<'tcx>(

// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let implied_bounds = match impl_ty.container {
ty::TraitContainer => FxHashSet::default(),
ty::ImplContainer => wfcheck::impl_implied_bounds(
tcx,
param_env,
container_id.expect_local(),
impl_ty_span,
),
};
let mut outlives_environment = OutlivesEnvironment::new(param_env);
outlives_environment.add_implied_bounds(&infcx, implied_bounds, impl_ty_hir_id);
outlives_environment.add_implied_bounds(&infcx, assumed_wf_types, impl_ty_hir_id);
infcx.check_region_obligations_and_report_errors(
impl_ty.def_id.expect_local(),
&outlives_environment,
Expand Down
Loading

0 comments on commit a9bb589

Please sign in to comment.