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

[WIP] some mir typeck cleanup #95763

Closed
wants to merge 6 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_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
locations: Locations,
category: ConstraintCategory,
) {
self.prove_predicates(
Some(ty::Binder::dummy(ty::PredicateKind::Trait(ty::TraitPredicate {
self.prove_predicate(
ty::Binder::dummy(ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: ty::ImplPolarity::Positive,
}))),
}))
.to_predicate(self.tcx()),
locations,
category,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
// }
// impl Foo for () {
// type Bar = ();
// fn foo(&self) ->&() {}
// fn foo(&self) -> &() {}
// }
// ```
// Both &Self::Bar and &() are WF
Expand Down
231 changes: 88 additions & 143 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ macro_rules! span_mirbug_and_err {
($context:expr, $elem:expr, $($message:tt)*) => ({
{
span_mirbug!($context, $elem, $($message)*);
$context.error()
$context.tcx().ty_error()
}
})
}
Expand Down Expand Up @@ -181,107 +181,69 @@ pub(crate) fn type_check<'mir, 'tcx>(
upvars,
};

let opaque_type_values = type_check_internal(
let mut checker = TypeChecker::new(
infcx,
param_env,
body,
promoted,
param_env,
&region_bound_pairs,
implicit_region_bound,
&mut borrowck_context,
|mut cx| {
cx.equate_inputs_and_outputs(&body, universal_regions, &normalized_inputs_and_output);
liveness::generate(
&mut cx,
body,
elements,
flow_inits,
move_data,
location_table,
use_polonius,
);

translate_outlives_facts(&mut cx);
let opaque_type_values =
infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();

opaque_type_values
.into_iter()
.map(|(opaque_type_key, decl)| {
cx.fully_perform_op(
Locations::All(body.span),
ConstraintCategory::OpaqueType,
CustomTypeOp::new(
|infcx| {
infcx.register_member_constraints(
param_env,
opaque_type_key,
decl.hidden_type.ty,
decl.hidden_type.span,
);
Ok(InferOk { value: (), obligations: vec![] })
},
|| "opaque_type_map".to_string(),
),
)
.unwrap();
let mut hidden_type = infcx.resolve_vars_if_possible(decl.hidden_type);
trace!(
"finalized opaque type {:?} to {:#?}",
opaque_type_key,
hidden_type.ty.kind()
);
if hidden_type.has_infer_types_or_consts() {
infcx.tcx.sess.delay_span_bug(
decl.hidden_type.span,
&format!("could not resolve {:#?}", hidden_type.ty.kind()),
);
hidden_type.ty = infcx.tcx.ty_error();
}

(opaque_type_key, (hidden_type, decl.origin))
})
.collect()
},
);

MirTypeckResults { constraints, universal_region_relations, opaque_type_values }
}
let mut verifier = TypeVerifier::new(&mut checker, promoted);
verifier.visit_body(&body);

#[instrument(
skip(infcx, body, promoted, region_bound_pairs, borrowck_context, extra),
level = "debug"
)]
fn type_check_internal<'a, 'tcx, R>(
infcx: &'a InferCtxt<'a, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
body: &'a Body<'tcx>,
promoted: &'a IndexVec<Promoted, Body<'tcx>>,
region_bound_pairs: &'a RegionBoundPairs<'tcx>,
implicit_region_bound: ty::Region<'tcx>,
borrowck_context: &'a mut BorrowCheckContext<'a, 'tcx>,
extra: impl FnOnce(TypeChecker<'a, 'tcx>) -> R,
) -> R {
let mut checker = TypeChecker::new(
infcx,
checker.typeck_mir(body);

checker.equate_inputs_and_outputs(&body, universal_regions, &normalized_inputs_and_output);
liveness::generate(
&mut checker,
body,
param_env,
region_bound_pairs,
implicit_region_bound,
borrowck_context,
elements,
flow_inits,
move_data,
location_table,
use_polonius,
);
let errors_reported = {
let mut verifier = TypeVerifier::new(&mut checker, promoted);
verifier.visit_body(&body);
verifier.errors_reported
};

if !errors_reported {
// if verifier failed, don't do further checks to avoid ICEs
checker.typeck_mir(body);
}
translate_outlives_facts(&mut checker);
let opaque_type_values = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
let opaque_type_values = opaque_type_values
.into_iter()
.map(|(opaque_type_key, decl)| {
checker
.fully_perform_op(
Locations::All(body.span),
ConstraintCategory::OpaqueType,
CustomTypeOp::new(
|infcx| {
infcx.register_member_constraints(
param_env,
opaque_type_key,
decl.hidden_type.ty,
decl.hidden_type.span,
);
Ok(InferOk { value: (), obligations: vec![] })
},
|| "opaque_type_map".to_string(),
),
)
.unwrap();
let mut hidden_type = infcx.resolve_vars_if_possible(decl.hidden_type);
trace!("finalized opaque type {:?} to {:#?}", opaque_type_key, hidden_type.ty.kind());
if hidden_type.has_infer_types_or_consts() {
infcx.tcx.sess.delay_span_bug(
decl.hidden_type.span,
&format!("could not resolve {:#?}", hidden_type.ty.kind()),
);
hidden_type.ty = infcx.tcx.ty_error();
}

extra(checker)
(opaque_type_key, (hidden_type, decl.origin))
})
.collect();

MirTypeckResults { constraints, universal_region_relations, opaque_type_values }
}

fn translate_outlives_facts(typeck: &mut TypeChecker<'_, '_>) {
Expand Down Expand Up @@ -323,14 +285,11 @@ enum FieldAccessError {

/// Verifies that MIR types are sane to not crash further checks.
///
/// The sanitize_XYZ methods here take an MIR object and compute its
/// type, calling `span_mirbug` and returning an error type if there
/// is a problem.
/// FIXME: Merge this with `TypeChecker`.
struct TypeVerifier<'a, 'b, 'tcx> {
cx: &'a mut TypeChecker<'b, 'tcx>,
promoted: &'b IndexVec<Promoted, Body<'tcx>>,
last_span: Span,
errors_reported: bool,
}

impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
Expand Down Expand Up @@ -387,34 +346,25 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
};
if let Some(uv) = maybe_uneval {
if let Some(promoted) = uv.promoted {
let check_err = |verifier: &mut TypeVerifier<'a, 'b, 'tcx>,
promoted: &Body<'tcx>,
ty,
san_ty| {
if let Err(terr) = verifier.cx.eq_types(
let promoted_body = &self.promoted[promoted];
self.sanitize_promoted(promoted_body, location);

let promoted_ty = promoted_body.return_ty();
if let Err(terr) = self.cx.eq_types(
ty,
promoted_ty,
location.to_locations(),
ConstraintCategory::Boring,
) {
span_mirbug!(
self,
promoted_body,
"bad promoted type ({:?}: {:?}): {:?}",
ty,
san_ty,
location.to_locations(),
ConstraintCategory::Boring,
) {
span_mirbug!(
verifier,
promoted,
"bad promoted type ({:?}: {:?}): {:?}",
ty,
san_ty,
terr
);
};
promoted_ty,
terr
);
};

if !self.errors_reported {
let promoted_body = &self.promoted[promoted];
self.sanitize_promoted(promoted_body, location);

let promoted_ty = promoted_body.return_ty();
check_err(self, promoted_body, ty, promoted_ty);
}
} else {
if let Err(terr) = self.cx.fully_perform_op(
location.to_locations(),
Expand Down Expand Up @@ -450,14 +400,12 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
}
}

if let ty::FnDef(def_id, substs) = *constant.literal.ty().kind() {
let instantiated_predicates = tcx.predicates_of(def_id).instantiate(tcx, substs);
self.cx.normalize_and_prove_instantiated_predicates(
def_id,
instantiated_predicates,
location.to_locations(),
);
}
self.cx.prove_predicate(
ty::Binder::dummy(ty::PredicateKind::WellFormed(constant.literal.ty().into()))
.to_predicate(self.tcx()),
location.to_locations(),
ConstraintCategory::Boring,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't we file a bug about this?

}
}

Expand Down Expand Up @@ -512,9 +460,6 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
for local_decl in &body.local_decls {
self.sanitize_type(local_decl, local_decl.ty);
}
if self.errors_reported {
return;
}
self.super_body(body);
}
}
Expand All @@ -524,7 +469,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
cx: &'a mut TypeChecker<'b, 'tcx>,
promoted: &'b IndexVec<Promoted, Body<'tcx>>,
) -> Self {
TypeVerifier { promoted, last_span: cx.body.span, cx, errors_reported: false }
TypeVerifier { promoted, last_span: cx.body.span, cx }
}

fn body(&self) -> &Body<'tcx> {
Expand Down Expand Up @@ -558,13 +503,21 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
for elem in place.projection.iter() {
if place_ty.variant_index.is_none() {
if place_ty.ty.references_error() {
assert!(self.errors_reported);
return PlaceTy::from_ty(self.tcx().ty_error());
}
}
place_ty = self.sanitize_projection(place_ty, elem, place, location);
}

self.cx.prove_predicate(
ty::Binder::dummy(ty::PredicateKind::WellFormed(
self.body().local_decls[place.local].ty.into(),
))
.to_predicate(self.tcx()),
location.to_locations(),
ConstraintCategory::Boring,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a known bug that is affected by this?

);

if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context {
let tcx = self.tcx();
let trait_ref = ty::TraitRef {
Expand Down Expand Up @@ -629,10 +582,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {

self.visit_body(&promoted_body);

if !self.errors_reported {
// if verifier failed, don't do further checks to avoid ICEs
self.cx.typeck_mir(promoted_body);
}
self.cx.typeck_mir(promoted_body);

self.cx.body = parent_body;
// Merge the outlives constraints back in, at the given location.
Expand Down Expand Up @@ -790,11 +740,6 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
}
}

fn error(&mut self) -> Ty<'tcx> {
self.errors_reported = true;
self.tcx().ty_error()
}

fn field_ty(
&mut self,
parent: &dyn fmt::Debug,
Expand Down Expand Up @@ -1926,7 +1871,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

&Rvalue::NullaryOp(_, ty) => {
&Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, ty) => {
let trait_ref = ty::TraitRef {
def_id: tcx.require_lang_item(LangItem::Sized, Some(self.last_span)),
substs: tcx.mk_substs_trait(ty, &[]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
infcx.tcx.struct_span_lint_hir(
lint::builtin::CONST_EVALUATABLE_UNCHECKED,
infcx.tcx.hir().local_def_id_to_hir_id(local_def_id),
span,
tcx.def_span(local_def_id),
|err| {
err.build("cannot use constants which depend on generic parameters in types")
.emit();
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2878,8 +2878,8 @@ where
#[stable(feature = "hash_extend_copy", since = "1.4.0")]
impl<'a, K, V, S> Extend<(&'a K, &'a V)> for HashMap<K, V, S>
where
K: Eq + Hash + Copy,
V: Copy,
K: Eq + Hash + Copy + 'a,
V: Copy + 'a,
S: BuildHasher,
{
#[inline]
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/borrowck/issue-7573.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub fn remove_package_from_database() {
lines_to_use.push(installed_id);
//~^ ERROR borrowed data escapes outside of closure
//~| NOTE `installed_id` escapes the closure body here
//~| NOTE requirement occurs because of a mutable reference to `Vec<&CrateId>`
//~| NOTE mutable references are invariant over their type parameter
};
list_database(push_id);

Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/borrowck/issue-7573.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ LL | let push_id = |installed_id: &CrateId| {
LL |
LL | lines_to_use.push(installed_id);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `installed_id` escapes the closure body here
|
= note: requirement occurs because of a mutable reference to `Vec<&CrateId>`
= note: mutable references are invariant over their type parameter
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: aborting due to previous error

Expand Down
Loading