Skip to content

correct spans in parem_env to deduplicate errors #102502

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

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
93 changes: 60 additions & 33 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::infer::outlives::env::OutlivesEnvironment;
use crate::infer::{InferCtxt, TyCtxtInferExt};
use crate::traits::error_reporting::InferCtxtExt as _;
use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
use rustc_data_structures::functor::IdFunctor;
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -216,11 +217,12 @@ pub fn type_known_to_meet_bound_modulo_regions<'a, 'tcx>(
#[instrument(level = "debug", skip(tcx, elaborated_env))]
fn do_normalize_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
cause: ObligationCause<'tcx>,
span: Span,
mut causes: impl Iterator<Item = ObligationCause<'tcx>> + Debug,
elaborated_env: ty::ParamEnv<'tcx>,
predicates: Vec<ty::Predicate<'tcx>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

From the use site, should we just take &mut?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, as mentioned above, fully_normalize requires taking ownership of the predicats. I could not think of a good way to deal with this for a while.

normalize_outlive_predicates: bool,
) -> Result<Vec<ty::Predicate<'tcx>>, ErrorGuaranteed> {
let span = cause.span;
// FIXME. We should really... do something with these region
// obligations. But this call just continues the older
// behavior (i.e., doesn't cause any new bugs), and it would
Expand All @@ -235,15 +237,27 @@ fn do_normalize_predicates<'tcx>(
// them here too, and we will remove this function when
// we move over to lazy normalization *anyway*.
tcx.infer_ctxt().ignoring_regions().enter(|infcx| {
let predicates = match fully_normalize(&infcx, cause, elaborated_env, predicates) {
Ok(predicates) => predicates,
let normalized_predicates = match predicates.try_map_id(|origin_predicate| {
if normalize_outlive_predicates
!= matches!(
origin_predicate.kind().skip_binder(),
ty::PredicateKind::TypeOutlives(..)
)
{
causes.next().unwrap();
Ok(origin_predicate)
} else {
fully_normalize(&infcx, causes.next().unwrap(), elaborated_env, origin_predicate)
}
}) {
Ok(normalized) => normalized,
Err(errors) => {
let reported = infcx.report_fulfillment_errors(&errors, None, false);
return Err(reported);
}
};

debug!("do_normalize_predictes: normalized predicates = {:?}", predicates);
debug!("do_normalize_predictes: normalized predicates = {:?}", normalized_predicates);

// We can use the `elaborated_env` here; the region code only
// cares about declarations like `'a: 'b`.
Expand All @@ -262,9 +276,17 @@ fn do_normalize_predicates<'tcx>(
);
}

match infcx.fully_resolve(predicates) {
Ok(predicates) => Ok(predicates),
Err(fixup_err) => {
normalized_predicates
.try_map_id(|predicate| {
if normalize_outlive_predicates
!= matches!(predicate.kind().skip_binder(), ty::PredicateKind::TypeOutlives(..))
{
Ok(predicate)
} else {
infcx.fully_resolve(predicate)
}
})
.map_err(|fixup_err| {
// If we encounter a fixup error, it means that some type
// variable wound up unconstrained. I actually don't know
// if this can happen, and I certainly don't expect it to
Expand All @@ -279,13 +301,13 @@ fn do_normalize_predicates<'tcx>(
"inference variables in normalized parameter environment: {}",
fixup_err
);
}
}
})
})
}

// FIXME: this is gonna need to be removed ...
/// Normalizes the parameter environment, reporting errors if they occur.
#[inline]
#[instrument(level = "debug", skip(tcx))]
pub fn normalize_param_env_or_error<'tcx>(
tcx: TyCtxt<'tcx>,
Expand All @@ -306,13 +328,30 @@ pub fn normalize_param_env_or_error<'tcx>(
// parameter environments once for every fn as it goes,
// and errors will get reported then; so outside of type inference we
// can be sure that no errors should occur.
let mut predicates: Vec<_> =
let predicates: Vec<_> =
util::elaborate_predicates(tcx, unnormalized_env.caller_bounds().into_iter())
.map(|obligation| obligation.predicate)
.collect();

debug!("normalize_param_env_or_error: elaborated-predicates={:?}", predicates);

normalize_param_env_with_causes(
tcx,
unnormalized_env,
cause.span,
std::iter::repeat(cause),
predicates,
)
}

#[instrument(level = "debug", skip(tcx))]
pub fn normalize_param_env_with_causes<'tcx>(
tcx: TyCtxt<'tcx>,
unnormalized_env: ty::ParamEnv<'tcx>,
span: Span,
causes: impl Iterator<Item = ObligationCause<'tcx>> + Clone + Debug,
predicates: Vec<ty::Predicate<'tcx>>,
) -> ty::ParamEnv<'tcx> {
let elaborated_env = ty::ParamEnv::new(
tcx.intern_predicates(&predicates),
unnormalized_env.reveal(),
Expand All @@ -337,53 +376,41 @@ pub fn normalize_param_env_or_error<'tcx>(
//
// This works fairly well because trait matching does not actually care about param-env
// TypeOutlives predicates - these are normally used by regionck.
let outlives_predicates: Vec<_> = predicates
.drain_filter(|predicate| {
matches!(predicate.kind().skip_binder(), ty::PredicateKind::TypeOutlives(..))
})
.collect();

debug!(
"normalize_param_env_or_error: predicates=(non-outlives={:?}, outlives={:?})",
predicates, outlives_predicates
);
let Ok(non_outlives_predicates) = do_normalize_predicates(
let Ok(predicates) = do_normalize_predicates(
tcx,
cause.clone(),
span.clone(),
causes.clone(),
elaborated_env,
predicates,
false,
) else {
// An unnormalized env is better than nothing.
debug!("normalize_param_env_or_error: errored resolving non-outlives predicates");
return elaborated_env;
};

debug!("normalize_param_env_or_error: non-outlives predicates={:?}", non_outlives_predicates);

// Not sure whether it is better to include the unnormalized TypeOutlives predicates
// here. I believe they should not matter, because we are ignoring TypeOutlives param-env
// predicates here anyway. Keeping them here anyway because it seems safer.
let outlives_env: Vec<_> =
non_outlives_predicates.iter().chain(&outlives_predicates).cloned().collect();
let outlives_env = ty::ParamEnv::new(
tcx.intern_predicates(&outlives_env),
tcx.intern_predicates(&predicates),
unnormalized_env.reveal(),
unnormalized_env.constness(),
);
let Ok(outlives_predicates) = do_normalize_predicates(
let Ok(predicates) = do_normalize_predicates(
tcx,
cause,
span,
causes,
outlives_env,
outlives_predicates,
predicates,
true
) else {
// An unnormalized env is better than nothing.
debug!("normalize_param_env_or_error: errored resolving outlives predicates");
return elaborated_env;
};
debug!("normalize_param_env_or_error: outlives predicates={:?}", outlives_predicates);

let mut predicates = non_outlives_predicates;
predicates.extend(outlives_predicates);
debug!("normalize_param_env_or_error: final predicates={:?}", predicates);
ty::ParamEnv::new(
tcx.intern_predicates(&predicates),
Expand Down
44 changes: 39 additions & 5 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::traits::util::elaborate_obligations;
use rustc_infer::traits::Obligation;
use rustc_middle::ty::{self, Binder, Predicate, PredicateKind, ToPredicate, Ty, TyCtxt};
use rustc_trait_selection::traits;

Expand Down Expand Up @@ -110,7 +112,7 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
}
// Compute the bounds on Self and the type parameters.

let ty::InstantiatedPredicates { mut predicates, .. } =
let ty::InstantiatedPredicates { mut predicates, spans } =
tcx.predicates_of(def_id).instantiate_identity(tcx);

// Finally, we have to normalize the bounds in the environment, in
Expand All @@ -132,6 +134,7 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {

let local_did = def_id.as_local();
let hir_id = local_did.map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id));
let mut is_fn = false;

let constness = match hir_id {
Some(hir_id) => match tcx.hir().get(hir_id) {
Expand Down Expand Up @@ -181,8 +184,11 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
kind:
hir::ItemKind::Fn(hir::FnSig { header: hir::FnHeader { constness, .. }, .. }, ..),
..
})
| hir::Node::TraitItem(hir::TraitItem {
}) => {
is_fn = true;
*constness
}
hir::Node::TraitItem(hir::TraitItem {
kind:
hir::TraitItemKind::Fn(
hir::FnSig { header: hir::FnHeader { constness, .. }, .. },
Expand Down Expand Up @@ -214,8 +220,36 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
_ => hir::CRATE_HIR_ID,
};

let cause = traits::ObligationCause::misc(tcx.def_span(def_id), body_id);
traits::normalize_param_env_or_error(tcx, unnormalized_env, cause)
let span = tcx.def_span(def_id);

if is_fn {
Copy link
Member

Choose a reason for hiding this comment

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

I'm skeptical that all this extra code is worthwhile... since all it's here for is to deduplicate a diagnostic that's still going to end up being an error. Especially because normalize_param_env_or_error is very delicate logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing it out. If that isn't worthwhile, could you take a look at the first commit, which is just a little optimization of param_env, and I hope it's acceptable?

Copy link
Member

@compiler-errors compiler-errors Oct 3, 2022

Choose a reason for hiding this comment

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

Are you sure this is an optimization in reality? Maybe you should put it up as a separate commit (separate from the span modifications) so we can do a perf run on it.

I am still not confident that the added complexity is worth it.

let mut spans = spans.into_iter();
let obligations: Vec<_> = predicates
.into_iter()
.map(|predicate| {
let cause = traits::ObligationCause::misc(spans.next().unwrap_or(span), body_id);
Obligation::new(cause, ty::ParamEnv::empty(), predicate)
})
.collect();

let (predicates, causes): (Vec<_>, Vec<_>) = elaborate_obligations(tcx, obligations)
.map(|obligation| (obligation.predicate, obligation.cause))
.unzip();

traits::normalize_param_env_with_causes(
tcx,
unnormalized_env,
span,
causes.into_iter(),
predicates,
)
} else {
traits::normalize_param_env_or_error(
tcx,
unnormalized_env,
traits::ObligationCause::misc(span, body_id),
)
}
}

/// Elaborate the environment.
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/associated-types/issue-102185.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: -Zdeduplicate-diagnostics=yes

fn f<T>(_: impl A<X = T>) {}
//~^ ERROR the trait bound `T: B` is not satisfied [E0277]
trait A: C<Z = <<Self as A>::X as B>::Y> {
type X: B;
}

trait B {
type Y;
}

trait C {
type Z;
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/associated-types/issue-102185.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0277]: the trait bound `T: B` is not satisfied
--> $DIR/issue-102185.rs:3:17
|
LL | fn f<T>(_: impl A<X = T>) {}
| ^^^^^^^^ the trait `B` is not implemented for `T`
|
help: consider restricting type parameter `T`
|
LL | fn f<T: B>(_: impl A<X = T>) {}
| +++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.