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

Fix PostBorrowckAnalysis for old solver #135899

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+173 −188
Open

Fix PostBorrowckAnalysis for old solver #135899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
153 changes: 33 additions & 120 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 @@ -17,20 +17,17 @@ use rustc_lint_defs::builtin::{
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::ty::error::TypeErrorToStringExt;
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{
AdtDef, BottomUpFolder, GenericArgKind, RegionKind, TypeFoldable, TypeSuperVisitable,
TypeVisitable, TypeVisitableExt, fold_regions,
AdtDef, GenericArgKind, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, fold_regions,
};
use rustc_session::lint::builtin::UNINHABITED_STATIC;
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::error_reporting::traits::on_unimplemented::OnUnimplementedDirective;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use tracing::{debug, instrument};
use ty::TypingMode;
use {rustc_attr_parsing as attr, rustc_hir as hir};

use super::compare_impl_item::check_type_bounds;
Expand Down Expand Up @@ -254,14 +251,18 @@ fn check_opaque_meets_bounds<'tcx>(
| hir::OpaqueTyOrigin::AsyncFn { parent, .. }
| hir::OpaqueTyOrigin::TyAlias { parent, .. } => parent,
};

let misc_cause = ObligationCause::misc(span, def_id);

// FIXME: We should reveal the TAITs that end up in where clauses here, otherwise we
// will not be able to match param-env candidates in the old solver, since we don't
// have eq-modulo-normalization. This is less of a problem than it seems, since this
// only matters if we have TAITs in where clauses, which isn't achievable with RPIT
// anyways.
let param_env = tcx.param_env(defining_use_anchor);

// FIXME(#132279): Once `PostBorrowckAnalysis` is supported in the old solver, this branch should be removed.
let infcx = tcx.infer_ctxt().build(if tcx.next_trait_solver_globally() {
TypingMode::post_borrowck_analysis(tcx, defining_use_anchor)
} else {
TypingMode::analysis_in_body(tcx, defining_use_anchor)
});
let infcx =
tcx.infer_ctxt().build(TypingMode::post_borrowck_analysis(tcx, defining_use_anchor));
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);

let args = match origin {
Expand All @@ -275,8 +276,6 @@ fn check_opaque_meets_bounds<'tcx>(
}),
};

let opaque_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);

// `ReErased` regions appear in the "parent_args" of closures/coroutines.
// We're ignoring them here and replacing them with fresh region variables.
// See tests in ui/type-alias-impl-trait/closure_{parent_args,wf_outlives}.rs.
Expand All @@ -289,19 +288,11 @@ fn check_opaque_meets_bounds<'tcx>(
_ => re,
});

// HACK: We eagerly instantiate some bounds to report better errors for them...
// This isn't necessary for correctness, since we register these bounds when
// equating the opaque below, but we should clean this up in the new solver.
// NOTE: We elaborate the explicit item bounds for better spans.
for (predicate, pred_span) in
tcx.explicit_item_bounds(def_id).iter_instantiated_copied(tcx, args)
traits::elaborate(tcx, tcx.explicit_item_bounds(def_id).iter_instantiated_copied(tcx, args))
{
let predicate = predicate.fold_with(&mut BottomUpFolder {
tcx,
ty_op: |ty| if ty == opaque_ty { hidden_ty } else { ty },
lt_op: |lt| lt,
ct_op: |ct| ct,
});

let predicate = ocx.normalize(&misc_cause, param_env, predicate);
ocx.register_obligation(Obligation::new(
tcx,
ObligationCause::new(
Expand All @@ -314,24 +305,24 @@ fn check_opaque_meets_bounds<'tcx>(
));
}

let misc_cause = ObligationCause::misc(span, def_id);
// FIXME: We should just register the item bounds here, rather than equating.
// FIXME(const_trait_impl): When we do that, please make sure to also register
// the `~const` bounds.
match ocx.eq(&misc_cause, param_env, opaque_ty, hidden_ty) {
Ok(()) => {}
Err(ty_err) => {
// Some types may be left "stranded" if they can't be reached
// from a lowered rustc_middle bound but they're mentioned in the HIR.
// This will happen, e.g., when a nested opaque is inside of a non-
// existent associated type, like `impl Trait<Missing = impl Trait>`.
// See <tests/ui/impl-trait/stranded-opaque.rs>.
let ty_err = ty_err.to_string(tcx);
let guar = tcx.dcx().span_delayed_bug(
span,
format!("could not unify `{hidden_ty}` with revealed type:\n{ty_err}"),
// And check the `~const` bounds for an RPIT.
if tcx.is_conditionally_const(def_id) {
for (predicate, pred_span) in tcx.const_conditions(def_id).instantiate(tcx, args) {
let predicate = ocx.normalize(
&misc_cause,
param_env,
predicate.to_host_effect_clause(tcx, ty::BoundConstness::Maybe),
);
return Err(guar);
ocx.register_obligation(Obligation::new(
tcx,
ObligationCause::new(
span,
def_id,
ObligationCauseCode::OpaqueTypeBound(pred_span, definition_def_id),
),
param_env,
predicate,
));
}
}

Expand All @@ -353,27 +344,7 @@ fn check_opaque_meets_bounds<'tcx>(
let wf_tys = ocx.assumed_wf_types_and_report_errors(param_env, defining_use_anchor)?;
ocx.resolve_regions_and_report_errors(defining_use_anchor, param_env, wf_tys)?;

if infcx.next_trait_solver() {
Ok(())
} else if let hir::OpaqueTyOrigin::FnReturn { .. } | hir::OpaqueTyOrigin::AsyncFn { .. } =
origin
{
// HACK: this should also fall through to the hidden type check below, but the original
// implementation had a bug where equivalent lifetimes are not identical. This caused us
// to reject existing stable code that is otherwise completely fine. The real fix is to
// compare the hidden types via our type equivalence/relation infra instead of doing an
// identity check.
let _ = infcx.take_opaque_types();
Ok(())
} else {
// Check that any hidden types found during wf checking match the hidden types that `type_of` sees.
for (mut key, mut ty) in infcx.take_opaque_types() {
ty.ty = infcx.resolve_vars_if_possible(ty.ty);
key = infcx.resolve_vars_if_possible(key);
sanity_check_found_hidden_type(tcx, key, ty)?;
}
Ok(())
}
Ok(())
}

fn best_definition_site_of_opaque<'tcx>(
Expand Down Expand Up @@ -461,50 +432,6 @@ fn best_definition_site_of_opaque<'tcx>(
}
}

fn sanity_check_found_hidden_type<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::OpaqueTypeKey<'tcx>,
mut ty: ty::OpaqueHiddenType<'tcx>,
) -> Result<(), ErrorGuaranteed> {
if ty.ty.is_ty_var() {
// Nothing was actually constrained.
return Ok(());
}
if let ty::Alias(ty::Opaque, alias) = ty.ty.kind() {
if alias.def_id == key.def_id.to_def_id() && alias.args == key.args {
// Nothing was actually constrained, this is an opaque usage that was
// only discovered to be opaque after inference vars resolved.
return Ok(());
}
}
let strip_vars = |ty: Ty<'tcx>| {
ty.fold_with(&mut BottomUpFolder {
tcx,
ty_op: |t| t,
ct_op: |c| c,
lt_op: |l| match l.kind() {
RegionKind::ReVar(_) => tcx.lifetimes.re_erased,
_ => l,
},
})
};
// Closures frequently end up containing erased lifetimes in their final representation.
// These correspond to lifetime variables that never got resolved, so we patch this up here.
ty.ty = strip_vars(ty.ty);
// Get the hidden type.
let hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args);
let hidden_ty = strip_vars(hidden_ty);

// If the hidden types differ, emit a type mismatch diagnostic.
if hidden_ty == ty.ty {
Ok(())
} else {
let span = tcx.def_span(key.def_id);
let other = ty::OpaqueHiddenType { ty: hidden_ty, span };
Err(ty.build_mismatch_error(&other, tcx)?.emit())
}
}

/// Check that the opaque's precise captures list is valid (if present).
/// We check this for regular `impl Trait`s and also RPITITs, even though the latter
/// are technically GATs.
Expand Down Expand Up @@ -1801,11 +1728,7 @@ pub(super) fn check_coroutine_obligations(

debug!(?typeck_results.coroutine_stalled_predicates);

let mode = if tcx.next_trait_solver_globally() {
TypingMode::post_borrowck_analysis(tcx, def_id)
} else {
TypingMode::analysis_in_body(tcx, def_id)
};
let mode = TypingMode::post_borrowck_analysis(tcx, def_id);

let infcx = tcx
.infer_ctxt()
Expand All @@ -1825,15 +1748,5 @@ pub(super) fn check_coroutine_obligations(
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
}

if !tcx.next_trait_solver_globally() {
// Check that any hidden types found when checking these stalled coroutine obligations
// are valid.
for (key, ty) in infcx.take_opaque_types() {
let hidden_type = infcx.resolve_vars_if_possible(ty);
let key = infcx.resolve_vars_if_possible(key);
sanity_check_found_hidden_type(tcx, key, hidden_type)?;
}
}

Ok(())
}
6 changes: 5 additions & 1 deletion compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,14 +711,18 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
// Already reported.
Overflow(OverflowError::Error(guar)) => {
self.set_tainted_by_errors(guar);
return guar
return guar;
},

Overflow(_) => {
bug!("overflow should be handled before the `report_selection_error` path");
}

SelectionError::ConstArgHasWrongType { ct, ct_ty, expected_ty } => {
if let Err(guar) = expected_ty.error_reported() {
return guar;
}

let mut diag = self.dcx().struct_span_err(
span,
format!("the constant `{ct}` is not of type `{expected_ty}`"),
Expand Down
79 changes: 51 additions & 28 deletions compiler/rustc_trait_selection/src/traits/normalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_infer::infer::at::At;
use rustc_infer::infer::{InferCtxt, InferOk};
use rustc_infer::infer::{InferCtxt, InferOk, RegionVariableOrigin};
use rustc_infer::traits::{
FromSolverError, Normalized, Obligation, PredicateObligations, TraitEngine,
};
Expand All @@ -11,7 +11,7 @@ use rustc_middle::span_bug;
use rustc_middle::traits::{ObligationCause, ObligationCauseCode};
use rustc_middle::ty::{
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, TypeVisitableExt,
TypingMode,
TypingMode, fold_regions,
};
use tracing::{debug, instrument};

Expand Down Expand Up @@ -127,11 +127,10 @@ pub(super) fn needs_normalization<'tcx, T: TypeVisitable<TyCtxt<'tcx>>>(
// Opaques are treated as rigid outside of `TypingMode::PostAnalysis`,
// so we can ignore those.
match infcx.typing_mode() {
// FIXME(#132279): We likely want to reveal opaques during post borrowck analysis
TypingMode::Coherence
| TypingMode::Analysis { .. }
| TypingMode::PostBorrowckAnalysis { .. } => flags.remove(ty::TypeFlags::HAS_TY_OPAQUE),
TypingMode::PostAnalysis => {}
TypingMode::Coherence | TypingMode::Analysis { .. } => {
flags.remove(ty::TypeFlags::HAS_TY_OPAQUE)
}
TypingMode::PostBorrowckAnalysis { .. } | TypingMode::PostAnalysis => {}
}

value.has_type_flags(flags)
Expand Down Expand Up @@ -169,6 +168,39 @@ impl<'a, 'b, 'tcx> AssocTypeNormalizer<'a, 'b, 'tcx> {

if !needs_normalization(self.selcx.infcx, &value) { value } else { value.fold_with(self) }
}

fn normalize_opaque_ty(&mut self, ty: Ty<'tcx>, data: ty::AliasTy<'tcx>) -> Ty<'tcx> {
let recursion_limit = self.cx().recursion_limit();
if !recursion_limit.value_within_limit(self.depth) {
self.selcx.infcx.err_ctxt().report_overflow_error(
OverflowCause::DeeplyNormalize(data.into()),
self.cause.span,
true,
|_| {},
);
}

let args = data.args.fold_with(self);
let generic_ty = self.cx().type_of(data.def_id);
let mut concrete_ty = generic_ty.instantiate(self.cx(), args);

if concrete_ty == ty {
Copy link
Member Author

Choose a reason for hiding this comment

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

This recursiveness check sucks, but it prevents bad error messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

could we move this check to type_of_opaque?

concrete_ty =
Ty::new_error_with_message(self.cx(), self.cause.span, "recursive opaque type");
}

let concrete_ty = fold_regions(self.cx(), concrete_ty, |re, _dbi| match re.kind() {
Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if we should consolidate all of these "replace erasing with infer" so that we only need to change it in one place when we start using a real binder...

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, we probably should 😁

ty::ReErased => self.selcx.infcx.next_region_var_in_universe(
Copy link
Member Author

Choose a reason for hiding this comment

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

:(((

Copy link
Member Author

Choose a reason for hiding this comment

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

pub fn unpeek<'a, F: FnMut(&mut ()) + 'a>(mut peek: F) -> impl FnMut(&mut ()) {
    move |i| peek(i)
}

pub fn test<P>(mut parser: P) -> impl FnMut(&mut ()) {
    unpeek(move |input: &mut ()| {
        let parser = &parser;
    })
}

I'll write up an analysis later.

RegionVariableOrigin::MiscVariable(self.cause.span),
ty::UniverseIndex::ROOT,
),
_ => re,
});
self.depth += 1;
let folded_ty = self.fold_ty(concrete_ty);
self.depth -= 1;
folded_ty
}
}

impl<'a, 'b, 'tcx> TypeFolder<TyCtxt<'tcx>> for AssocTypeNormalizer<'a, 'b, 'tcx> {
Expand Down Expand Up @@ -223,29 +255,20 @@ impl<'a, 'b, 'tcx> TypeFolder<TyCtxt<'tcx>> for AssocTypeNormalizer<'a, 'b, 'tcx
ty::Opaque => {
// Only normalize `impl Trait` outside of type inference, usually in codegen.
match self.selcx.infcx.typing_mode() {
// FIXME(#132279): We likely want to reveal opaques during post borrowck analysis
TypingMode::Coherence
| TypingMode::Analysis { .. }
| TypingMode::PostBorrowckAnalysis { .. } => ty.super_fold_with(self),
TypingMode::PostAnalysis => {
let recursion_limit = self.cx().recursion_limit();
if !recursion_limit.value_within_limit(self.depth) {
self.selcx.infcx.err_ctxt().report_overflow_error(
OverflowCause::DeeplyNormalize(data.into()),
self.cause.span,
true,
|_| {},
);
TypingMode::Coherence | TypingMode::Analysis { .. } => ty.super_fold_with(self),
TypingMode::PostBorrowckAnalysis { defined_opaque_types } => {
if data
.def_id
.as_local()
.is_some_and(|def_id| defined_opaque_types.contains(&def_id))
{
self.normalize_opaque_ty(ty, data)
} else {
// Treat non-defining opaques as rigid
ty.super_fold_with(self)
}

let args = data.args.fold_with(self);
let generic_ty = self.cx().type_of(data.def_id);
let concrete_ty = generic_ty.instantiate(self.cx(), args);
self.depth += 1;
let folded_ty = self.fold_ty(concrete_ty);
self.depth -= 1;
folded_ty
}
TypingMode::PostAnalysis => self.normalize_opaque_ty(ty, data),
}
}

Expand Down
Loading
Loading