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

some trait system cleanups #120463

Merged
merged 5 commits into from
Feb 19, 2024
Merged

some trait system cleanups #120463

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
486c7b6
never normalize without eager inference replacement
lcnr Jan 29, 2024
399a258
remove `pred_known_to_hold_modulo_regions`
lcnr Jan 29, 2024
df55f56
`normalize_projection_ty` is not used with next-solver
lcnr Feb 5, 2024
9771fb0
split `project` into multiple files
lcnr Feb 5, 2024
0c7672a
remove outdated comment
lcnr Feb 5, 2024
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
50 changes: 3 additions & 47 deletions compiler/rustc_trait_selection/src/traits/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,60 +119,16 @@ pub fn predicates_for_generics<'tcx>(

/// Determines whether the type `ty` is known to meet `bound` and
/// returns true if so. Returns false if `ty` either does not meet
/// `bound` or is not known to meet bound (note that this is
/// conservative towards *no impl*, which is the opposite of the
/// `evaluate` methods).
/// `bound` or is not known to meet bound.
pub fn type_known_to_meet_bound_modulo_regions<'tcx>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
def_id: DefId,
) -> bool {
let trait_ref = ty::TraitRef::new(infcx.tcx, def_id, [ty]);
pred_known_to_hold_modulo_regions(infcx, param_env, trait_ref)
}

/// FIXME(@lcnr): this function doesn't seem right and shouldn't exist?
///
/// Ping me on zulip if you want to use this method and need help with finding
/// an appropriate replacement.
#[instrument(level = "debug", skip(infcx, param_env, pred), ret)]
fn pred_known_to_hold_modulo_regions<'tcx>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
pred: impl ToPredicate<'tcx>,
) -> bool {
let obligation = Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, pred);

let result = infcx.evaluate_obligation_no_overflow(&obligation);
debug!(?result);

if result.must_apply_modulo_regions() {
true
} else if result.may_apply() {
// Sometimes obligations are ambiguous because the recursive evaluator
// is not smart enough, so we fall back to fulfillment when we're not certain
// that an obligation holds or not. Even still, we must make sure that
// the we do no inference in the process of checking this obligation.
let goal = infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
infcx.probe(|_| {
let ocx = ObligationCtxt::new(infcx);
ocx.register_obligation(obligation);

let errors = ocx.select_all_or_error();
match errors.as_slice() {
// Only known to hold if we did no inference.
[] => infcx.shallow_resolve(goal) == goal,

errors => {
debug!(?errors);
false
}
}
})
} else {
false
}
let obligation = Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, trait_ref);
infcx.predicate_must_hold_modulo_regions(&obligation)
Copy link
Member

Choose a reason for hiding this comment

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

There's a fair chance this will cause regression bc we're no longer falling back to fulfillment in the local infcx lol

Copy link
Contributor Author

@lcnr lcnr Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we could do a crater run? think it's fine though, reverting this is straightforward 😀

}

#[instrument(level = "debug", skip(tcx, elaborated_env))]
Expand Down