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

Remove fully_normalize #127631

Merged
merged 1 commit into from
Jul 13, 2024
Merged
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
27 changes: 14 additions & 13 deletions compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use rustc_middle::ty::print::{PrintTraitPredicateExt as _, TraitPredPrintModifie
use rustc_middle::ty::{self, fold::BottomUpFolder, Ty, TypeFoldable};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::{symbol::kw, Span};
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCtxt};

use crate::{LateContext, LateLintPass, LintContext};

Expand Down Expand Up @@ -130,24 +129,26 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound {
.iter_instantiated_copied(cx.tcx, proj.projection_term.args)
{
let assoc_pred = assoc_pred.fold_with(proj_replacer);
let Ok(assoc_pred) = traits::fully_normalize(
infcx,

let ocx = ObligationCtxt::new(infcx);
let assoc_pred =
ocx.normalize(&traits::ObligationCause::dummy(), cx.param_env, assoc_pred);
if !ocx.select_all_or_error().is_empty() {
// Can't normalize for some reason...?
continue;
}

ocx.register_obligation(traits::Obligation::new(
cx.tcx,
traits::ObligationCause::dummy(),
cx.param_env,
assoc_pred,
) else {
continue;
};
));

// If that predicate doesn't hold modulo regions (but passed during type-check),
// then we must've taken advantage of the hack in `project_and_unify_types` where
// we replace opaques with inference vars. Emit a warning!
if !infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new(
cx.tcx,
traits::ObligationCause::dummy(),
cx.param_env,
assoc_pred,
)) {
if !ocx.select_all_or_error().is_empty() {
// If it's a trait bound and an opaque that doesn't satisfy it,
// then we can emit a suggestion to add the bound.
let add_bound = match (proj_term.kind(), assoc_pred.kind().skip_binder()) {
Expand Down
46 changes: 8 additions & 38 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,13 +271,14 @@ fn do_normalize_predicates<'tcx>(
// them here too, and we will remove this function when
// we move over to lazy normalization *anyway*.
let infcx = tcx.infer_ctxt().ignoring_regions().build();
let predicates = match fully_normalize(&infcx, cause, elaborated_env, predicates) {
Ok(predicates) => predicates,
Err(errors) => {
let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
return Err(reported);
}
};
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
let predicates = ocx.normalize(&cause, elaborated_env, predicates);

let errors = ocx.select_all_or_error();
if !errors.is_empty() {
let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
return Err(reported);
}

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

Expand Down Expand Up @@ -465,37 +466,6 @@ pub fn normalize_param_env_or_error<'tcx>(
ty::ParamEnv::new(tcx.mk_clauses(&predicates), unnormalized_env.reveal())
}

/// Normalize a type and process all resulting obligations, returning any errors.
///
/// FIXME(-Znext-solver): This should be replaced by `At::deeply_normalize`
/// which has the same behavior with the new solver. Because using a separate
/// fulfillment context worsens caching in the old solver, `At::deeply_normalize`
/// is still lazy with the old solver as it otherwise negatively impacts perf.
#[instrument(skip_all)]
pub fn fully_normalize<'tcx, T>(
infcx: &InferCtxt<'tcx>,
cause: ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: T,
) -> Result<T, Vec<FulfillmentError<'tcx>>>
where
T: TypeFoldable<TyCtxt<'tcx>>,
{
let ocx = ObligationCtxt::new_with_diagnostics(infcx);
debug!(?value);
let normalized_value = ocx.normalize(&cause, param_env, value);
debug!(?normalized_value);
debug!("select_all_or_error start");
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
return Err(errors);
}
debug!("select_all_or_error complete");
let resolved_value = infcx.resolve_vars_if_possible(normalized_value);
debug!(?resolved_value);
Ok(resolved_value)
}

/// Normalizes the predicates and checks whether they hold in an empty environment. If this
/// returns true, then either normalize encountered an error or one of the predicates did not
/// hold. Used when creating vtables to check for unsatisfiable methods.
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_trait_selection/src/traits/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,9 @@ impl<'tcx> At<'_, 'tcx> {
/// same goals in both a temporary and the shared context which negatively impacts
/// performance as these don't share caching.
///
/// FIXME(-Znext-solver): This has the same behavior as `traits::fully_normalize`
/// in the new solver, but because of performance reasons, we currently reuse an
/// existing fulfillment context in the old solver. Once we also eagerly prove goals with
/// the old solver or have removed the old solver, remove `traits::fully_normalize` and
/// rename this function to `At::fully_normalize`.
/// FIXME(-Znext-solver): For performance reasons, we currently reuse an existing
/// fulfillment context in the old solver. Once we have removed the old solver, we
/// can remove the `fulfill_cx` parameter on this function.
fn deeply_normalize<T, E>(
self,
value: T,
Expand Down
29 changes: 11 additions & 18 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ use crate::error_reporting::traits::to_pretty_impl_header;
use crate::errors::NegativePositiveConflict;
use crate::infer::{InferCtxt, InferOk, TyCtxtInferExt};
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::{
self, coherence, FutureCompatOverlapErrorKind, ObligationCause, ObligationCtxt,
};
use crate::traits::{coherence, FutureCompatOverlapErrorKind, ObligationCause, ObligationCtxt};
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{codes::*, Diag, EmissionGuarantee};
use rustc_hir::def_id::{DefId, LocalDefId};
Expand Down Expand Up @@ -219,19 +217,17 @@ fn fulfill_implication<'tcx>(
param_env, source_trait_ref, target_impl
);

let source_trait_ref =
match traits::fully_normalize(infcx, ObligationCause::dummy(), param_env, source_trait_ref)
{
Ok(source_trait_ref) => source_trait_ref,
Err(_errors) => {
infcx.dcx().span_delayed_bug(
infcx.tcx.def_span(source_impl),
format!("failed to fully normalize {source_trait_ref}"),
);
source_trait_ref
}
};
let ocx = ObligationCtxt::new(infcx);
let source_trait_ref = ocx.normalize(&ObligationCause::dummy(), param_env, source_trait_ref);

if !ocx.select_all_or_error().is_empty() {
infcx.dcx().span_delayed_bug(
infcx.tcx.def_span(source_impl),
format!("failed to fully normalize {source_trait_ref}"),
);
}
Comment on lines +223 to +228
Copy link
Contributor

Choose a reason for hiding this comment

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

does anything break if we remove this check and just return Err(()) if normalization fails?


let source_trait_ref = infcx.resolve_vars_if_possible(source_trait_ref);
let source_trait = ImplSubject::Trait(source_trait_ref);

let selcx = SelectionContext::new(infcx);
Expand All @@ -253,9 +249,6 @@ fn fulfill_implication<'tcx>(
return Err(());
};

// Needs to be `in_snapshot` because this function is used to rebase
// generic parameters, which may happen inside of a select within a probe.
let ocx = ObligationCtxt::new(infcx);
Copy link
Member Author

Choose a reason for hiding this comment

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

Reusing the ocx from above, since it should be empty again.

// attempt to prove all of the predicates for impl2 given those for impl1
// (which are packed up in penv)
ocx.register_obligations(obligations.chain(more_obligations));
Expand Down
Loading