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.

Sign up for GitHub

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

Jump to bottom

cleanup our region error API #110220

Merged
merged 3 commits into from
Apr 13, 2023
Merged

cleanup our region error API #110220

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a19adef
region error cleanup
lcnr Apr 12, 2023
cc82ccb
`#[must_use]` for fns returning a list of errors
lcnr Apr 12, 2023
c0d3d32
fix comment
lcnr Apr 12, 2023
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: 2 additions & 5 deletions compiler/rustc_hir_analysis/src/check/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,8 @@ fn check_opaque_meets_bounds<'tcx>(
hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) => {}
// Can have different predicates to their defining use
hir::OpaqueTyOrigin::TyAlias => {
let outlives_environment = OutlivesEnvironment::new(param_env);
let _ = infcx.err_ctxt().check_region_obligations_and_report_errors(
defining_use_anchor,
&outlives_environment,
);
let outlives_env = OutlivesEnvironment::new(param_env);
let _ = ocx.resolve_regions_and_report_errors(defining_use_anchor, &outlives_env);
}
}
// Clean up after ourselves
Expand Down
40 changes: 11 additions & 29 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,6 @@ fn compare_method_predicate_entailment<'tcx>(
param_env,
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()),
);
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
Expand Down Expand Up @@ -722,18 +718,18 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
return Err(reported);
}

let collected_types = collector.types;

// 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,
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys),
);
infcx
.err_ctxt()
.check_region_obligations_and_report_errors(impl_m_def_id, &outlives_environment)?;
ocx.resolve_regions_and_report_errors(impl_m_def_id, &outlives_env)?;

let mut collected_tys = FxHashMap::default();
for (def_id, (ty, substs)) in collector.types {
for (def_id, (ty, substs)) in collected_types {
match infcx.fully_resolve(ty) {
Ok(ty) => {
// `ty` contains free regions that we created earlier while liberating the
Expand Down Expand Up @@ -1742,11 +1738,8 @@ pub(super) fn compare_impl_const_raw(
return Err(infcx.err_ctxt().report_fulfillment_errors(&errors));
}

let outlives_environment = OutlivesEnvironment::new(param_env);
infcx
.err_ctxt()
.check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment)?;
Ok(())
let outlives_env = OutlivesEnvironment::new(param_env);
ocx.resolve_regions_and_report_errors(impl_const_item_def, &outlives_env)
}

pub(super) fn compare_impl_ty<'tcx>(
Expand Down Expand Up @@ -1845,13 +1838,8 @@ fn compare_type_predicate_entailment<'tcx>(

// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let outlives_environment = OutlivesEnvironment::new(param_env);
infcx.err_ctxt().check_region_obligations_and_report_errors(
impl_ty.def_id.expect_local(),
&outlives_environment,
)?;

Ok(())
let outlives_env = OutlivesEnvironment::new(param_env);
ocx.resolve_regions_and_report_errors(impl_ty_def_id, &outlives_env)
}

/// Validate that `ProjectionCandidate`s created for this associated type will
Expand Down Expand Up @@ -2063,14 +2051,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 outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);

infcx.err_ctxt().check_region_obligations_and_report_errors(
impl_ty.def_id.expect_local(),
&outlives_environment,
)?;

Ok(())
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
ocx.resolve_regions_and_report_errors(impl_ty_def_id, &outlives_env)
}

fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str {
Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,9 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
return;
}

let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);

let _ = infcx
.err_ctxt()
.check_region_obligations_and_report_errors(body_def_id, &outlives_environment);
let _ = wfcx.ocx.resolve_regions_and_report_errors(body_def_id, &outlives_env);
}

fn check_well_formed(tcx: TyCtxt<'_>, def_id: hir::OwnerId) {
Expand Down Expand Up @@ -680,12 +678,7 @@ fn resolve_regions_with_wf_tys<'tcx>(

add_constraints(&infcx, region_bound_pairs);

infcx.process_registered_region_obligations(
outlives_environment.region_bound_pairs(),
param_env,
);
let errors = infcx.resolve_regions(&outlives_environment);

debug!(?errors, "errors");

// If we were able to prove that the type outlives the region without
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_hir_analysis/src/coherence/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,7 @@ fn visit_implementation_of_dispatch_from_dyn(tcx: TyCtxt<'_>, impl_did: LocalDef

// Finally, resolve all regions.
let outlives_env = OutlivesEnvironment::new(param_env);
let _ = infcx
.err_ctxt()
.check_region_obligations_and_report_errors(impl_did, &outlives_env);
let _ = ocx.resolve_regions_and_report_errors(impl_did, &outlives_env);
}
}
_ => {
Expand Down Expand Up @@ -592,7 +590,7 @@ pub fn coerce_unsized_info<'tcx>(tcx: TyCtxt<'tcx>, impl_did: LocalDefId) -> Coe

// Finally, resolve all regions.
let outlives_env = OutlivesEnvironment::new(param_env);
let _ = infcx.err_ctxt().check_region_obligations_and_report_errors(impl_did, &outlives_env);
let _ = ocx.resolve_regions_and_report_errors(impl_did, &outlives_env);

CoerceUnsizedInfo { custom_kind: kind }
}
3 changes: 1 addition & 2 deletions compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ fn get_impl_substs(

let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let _ =
infcx.err_ctxt().check_region_obligations_and_report_errors(impl1_def_id, &outlives_env);
let _ = ocx.resolve_regions_and_report_errors(impl1_def_id, &outlives_env);
let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
let span = tcx.def_span(impl1_def_id);
tcx.sess.emit_err(SubstsOnOverriddenImpl { span });
Expand Down
30 changes: 28 additions & 2 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ use rustc_middle::ty::{
self, error::TypeError, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable,
TypeVisitable, TypeVisitableExt,
};
use rustc_span::DUMMY_SP;
use rustc_span::{sym, symbol::kw, BytePos, DesugaringKind, Pos, Span};
use rustc_target::spec::abi;
use std::ops::{ControlFlow, Deref};
Expand Down Expand Up @@ -113,7 +114,11 @@ fn escape_literal(s: &str) -> String {

/// A helper for building type related errors. The `typeck_results`
/// field is only populated during an in-progress typeck.
/// Get an instance by calling `InferCtxt::err` or `FnCtxt::infer_err`.
/// Get an instance by calling `InferCtxt::err_ctxt` or `FnCtxt::err_ctxt`.
///
/// You must only create this if you intend to actually emit an error.
/// This provides a lot of utility methods which should not be used
/// during the happy path.
pub struct TypeErrCtxt<'a, 'tcx> {
pub infcx: &'a InferCtxt<'tcx>,
pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
Expand All @@ -125,6 +130,19 @@ pub struct TypeErrCtxt<'a, 'tcx> {
Box<dyn Fn(Ty<'tcx>) -> Vec<(Ty<'tcx>, Vec<PredicateObligation<'tcx>>)> + 'a>,
}

impl Drop for TypeErrCtxt<'_, '_> {
fn drop(&mut self) {
if let Some(_) = self.infcx.tcx.sess.has_errors_or_delayed_span_bugs() {
// ok, emitted an error.
} else {
self.infcx
Copy link
Member

Choose a reason for hiding this comment

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

This might create a problem if we create an error then cancel it, but I guess we can just wait until the ICE is filed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, imo that should not happen. Once you start using the err_ctxt() you should only cancel if we already emit an error somewhere else.

Copy link
Member

Choose a reason for hiding this comment

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

I think this caused rust-lang/rust-clippy#10645 🙈

.tcx
.sess
.delay_span_bug(DUMMY_SP, "used a `TypeErrCtxt` without failing compilation");
}
}
}

impl TypeErrCtxt<'_, '_> {
/// This is just to avoid a potential footgun of accidentally
/// dropping `typeck_results` by calling `InferCtxt::err_ctxt`
Expand Down Expand Up @@ -419,7 +437,11 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
&self,
generic_param_scope: LocalDefId,
errors: &[RegionResolutionError<'tcx>],
) {
) -> ErrorGuaranteed {
if let Some(guaranteed) = self.infcx.tainted_by_errors() {
return guaranteed;
}

debug!("report_region_errors(): {} errors to start", errors.len());

// try to pre-process the errors, which will group some of them
Expand Down Expand Up @@ -499,6 +521,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
}
}

self.tcx
.sess
.delay_span_bug(self.tcx.def_span(generic_param_scope), "expected region errors")
}

// This method goes through all the errors and try to group certain types
Expand Down
142 changes: 1 addition & 141 deletions compiler/rustc_infer/src/infer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ use self::combine::CombineFields;
use self::error_reporting::TypeErrCtxt;
use self::free_regions::RegionRelations;
use self::lexical_region_resolve::LexicalRegionResolutions;
use self::outlives::env::OutlivesEnvironment;
use self::region_constraints::{GenericKind, RegionConstraintData, VarInfos, VerifyBound};
use self::region_constraints::{GenericKind, VarInfos, VerifyBound};
use self::region_constraints::{
RegionConstraintCollector, RegionConstraintStorage, RegionSnapshot,
};
Expand Down Expand Up @@ -1213,95 +1212,6 @@ impl<'tcx> InferCtxt<'tcx> {
self.tainted_by_errors.set(Some(e));
}

pub fn skip_region_resolution(&self) {
let (var_infos, _) = {
let mut inner = self.inner.borrow_mut();
let inner = &mut *inner;
// Note: `inner.region_obligations` may not be empty, because we
// didn't necessarily call `process_registered_region_obligations`.
// This is okay, because that doesn't introduce new vars.
inner
.region_constraint_storage
.take()
.expect("regions already resolved")
.with_log(&mut inner.undo_log)
.into_infos_and_data()
};

let lexical_region_resolutions = LexicalRegionResolutions {
values: rustc_index::vec::IndexVec::from_elem_n(
crate::infer::lexical_region_resolve::VarValue::Value(self.tcx.lifetimes.re_erased),
var_infos.len(),
),
};

let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
assert!(old_value.is_none());
}

/// Process the region constraints and return any errors that
/// result. After this, no more unification operations should be
/// done -- or the compiler will panic -- but it is legal to use
/// `resolve_vars_if_possible` as well as `fully_resolve`.
pub fn resolve_regions(
&self,
outlives_env: &OutlivesEnvironment<'tcx>,
) -> Vec<RegionResolutionError<'tcx>> {
let (var_infos, data) = {
let mut inner = self.inner.borrow_mut();
let inner = &mut *inner;
assert!(
self.tainted_by_errors().is_some() || inner.region_obligations.is_empty(),
"region_obligations not empty: {:#?}",
inner.region_obligations
);
inner
.region_constraint_storage
.take()
.expect("regions already resolved")
.with_log(&mut inner.undo_log)
.into_infos_and_data()
};

let region_rels = &RegionRelations::new(self.tcx, outlives_env.free_region_map());

let (lexical_region_resolutions, errors) =
lexical_region_resolve::resolve(outlives_env.param_env, region_rels, var_infos, data);

let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
assert!(old_value.is_none());

errors
}
/// Obtains (and clears) the current set of region
/// constraints. The inference context is still usable: further
/// unifications will simply add new constraints.
///
/// This method is not meant to be used with normal lexical region
/// resolution. Rather, it is used in the NLL mode as a kind of
/// interim hack: basically we run normal type-check and generate
/// region constraints as normal, but then we take them and
/// translate them into the form that the NLL solver
/// understands. See the NLL module for mode details.
pub fn take_and_reset_region_constraints(&self) -> RegionConstraintData<'tcx> {
assert!(
self.inner.borrow().region_obligations.is_empty(),
"region_obligations not empty: {:#?}",
self.inner.borrow().region_obligations
);

self.inner.borrow_mut().unwrap_region_constraints().take_and_reset_data()
}

/// Gives temporary access to the region constraint data.
pub fn with_region_constraints<R>(
&self,
op: impl FnOnce(&RegionConstraintData<'tcx>) -> R,
) -> R {
let mut inner = self.inner.borrow_mut();
op(inner.unwrap_region_constraints().data())
}

pub fn region_var_origin(&self, vid: ty::RegionVid) -> RegionVariableOrigin {
let mut inner = self.inner.borrow_mut();
let inner = &mut *inner;
Expand Down Expand Up @@ -1754,56 +1664,6 @@ impl<'cx, 'tcx> Drop for CanonicalizationCtxtGuard<'cx, 'tcx> {
}

impl<'tcx> TypeErrCtxt<'_, 'tcx> {
/// Processes registered region obliations and resolves regions, reporting
/// any errors if any were raised. Prefer using this function over manually
/// calling `resolve_regions_and_report_errors`.
pub fn check_region_obligations_and_report_errors(
&self,
generic_param_scope: LocalDefId,
outlives_env: &OutlivesEnvironment<'tcx>,
) -> Result<(), ErrorGuaranteed> {
self.process_registered_region_obligations(
outlives_env.region_bound_pairs(),
outlives_env.param_env,
);

self.resolve_regions_and_report_errors(generic_param_scope, outlives_env)
}

/// Process the region constraints and report any errors that
/// result. After this, no more unification operations should be
/// done -- or the compiler will panic -- but it is legal to use
/// `resolve_vars_if_possible` as well as `fully_resolve`.
///
/// Make sure to call [`InferCtxt::process_registered_region_obligations`]
/// first, or preferably use [`TypeErrCtxt::check_region_obligations_and_report_errors`]
/// to do both of these operations together.
pub fn resolve_regions_and_report_errors(
&self,
generic_param_scope: LocalDefId,
outlives_env: &OutlivesEnvironment<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let errors = self.resolve_regions(outlives_env);

if let None = self.tainted_by_errors() {
// As a heuristic, just skip reporting region errors
// altogether if other errors have been reported while
// this infcx was in use. This is totally hokey but
// otherwise we have a hard time separating legit region
// errors from silly ones.
self.report_region_errors(generic_param_scope, &errors);
}

if errors.is_empty() {
Ok(())
} else {
Err(self
.tcx
.sess
.delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted"))
}
}

// [Note-Type-error-reporting]
// An invariant is that anytime the expected or actual type is Error (the special
// error type, meaning that an error occurred when typechecking this expression),
Expand Down
Loading