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

Eagerly normalize HIR paths in new trait solver #113473

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
7 changes: 5 additions & 2 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1317,7 +1317,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Find the type of `e`. Supply hints based on the type we are casting to,
// if appropriate.
let t_cast = self.to_ty_saving_user_provided_ty(t);
let t_cast = self.resolve_vars_if_possible(t_cast);
// FIXME(-Ztrait-solver): This structural resolve is only here for diagnostics,
// because we use the TyKind to early return from `CastCheck::new`.
let t_cast = self.try_structurally_resolve_type(t.span, t_cast);
let t_expr = self.check_expr_with_expectation(e, ExpectCastableToType(t_cast));
let t_expr = self.resolve_vars_if_possible(t_expr);

Expand Down Expand Up @@ -3142,7 +3144,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fields: &[Ident],
expr: &'tcx hir::Expr<'tcx>,
) -> Ty<'tcx> {
let container = self.to_ty(container).normalized;
// We only need to normalize in the old solver
let container = self.normalize(expr.span, self.to_ty(container));
Copy link
Contributor

Choose a reason for hiding this comment

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

do we not normalize in the structurally_resolve_type inside of the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, structurally resolve only processes obligations in the old solver


let mut field_indices = Vec::with_capacity(fields.len());
let mut current_container = container;
Expand Down
89 changes: 55 additions & 34 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::errors::CtorIsPrivate;
use crate::method::{self, MethodCallee, SelfSource};
use crate::rvalue_scopes;
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, RawTy};
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt};
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, Diagnostic, ErrorGuaranteed, MultiSpan, StashKey};
Expand Down Expand Up @@ -324,6 +324,26 @@
)
}

// FIXME(-Ztrait-solver=next): This could be replaced with `try_structurally_resolve`
// calls after the new trait solver is stable.
/// TODO: I don't know what to call this.

Check failure on line 329 in compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

TODO is used for tasks that should be done before merging a PR; If you want to leave a message in the codebase use FIXME
Copy link
Contributor

Choose a reason for hiding this comment

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

why is that method needed 🤔 because we don't want to normalize with the old solver here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because we need to normalize with the old solver here. This one calls the version of normalize that does both.

pub(super) fn structurally_normalize_after_astconv(
&self,
span: Span,
value: Ty<'tcx>,
) -> Ty<'tcx> {
match self
.at(&self.misc(span), self.param_env)
.structurally_normalize(value, &mut **self.inh.fulfillment_cx.borrow_mut())
{
Ok(ty) => ty,
Err(errors) => {
let guar = self.err_ctxt().report_fulfillment_errors(&errors);
Ty::new_error(self.tcx, guar)
}
}
}

pub fn require_type_meets(
&self,
ty: Ty<'tcx>,
Expand Down Expand Up @@ -374,37 +394,33 @@
}
}

pub fn handle_raw_ty(&self, span: Span, ty: Ty<'tcx>) -> RawTy<'tcx> {
RawTy { raw: ty, normalized: self.normalize(span, ty) }
}

pub fn to_ty(&self, ast_t: &hir::Ty<'_>) -> RawTy<'tcx> {
pub fn to_ty(&self, ast_t: &hir::Ty<'_>) -> Ty<'tcx> {
let t = self.astconv().ast_ty_to_ty(ast_t);
self.register_wf_obligation(t.into(), ast_t.span, traits::WellFormed(None));
self.handle_raw_ty(ast_t.span, t)
t
}

pub fn to_ty_saving_user_provided_ty(&self, ast_ty: &hir::Ty<'_>) -> Ty<'tcx> {
let ty = self.to_ty(ast_ty);
debug!("to_ty_saving_user_provided_ty: ty={:?}", ty);

if Self::can_contain_user_lifetime_bounds(ty.raw) {
let c_ty = self.canonicalize_response(UserType::Ty(ty.raw));
if Self::can_contain_user_lifetime_bounds(ty) {
let c_ty = self.canonicalize_response(UserType::Ty(ty));
debug!("to_ty_saving_user_provided_ty: c_ty={:?}", c_ty);
self.typeck_results.borrow_mut().user_provided_types_mut().insert(ast_ty.hir_id, c_ty);
}

ty.normalized
self.normalize(ast_ty.span, ty)
}

pub(super) fn user_args_for_adt(ty: RawTy<'tcx>) -> UserArgs<'tcx> {
match (ty.raw.kind(), ty.normalized.kind()) {
pub(super) fn user_args_for_adt(raw: Ty<'tcx>, normalized: Ty<'tcx>) -> UserArgs<'tcx> {
match (raw.kind(), normalized.kind()) {
(ty::Adt(_, args), _) => UserArgs { args, user_self_ty: None },
(_, ty::Adt(adt, args)) => UserArgs {
args,
user_self_ty: Some(UserSelfTy { impl_def_id: adt.did(), self_ty: ty.raw }),
user_self_ty: Some(UserSelfTy { impl_def_id: adt.did(), self_ty: raw }),
},
_ => bug!("non-adt type {:?}", ty),
_ => bug!("non-adt type {:?}", raw),
}
}

Expand Down Expand Up @@ -817,7 +833,7 @@
qpath: &'tcx QPath<'tcx>,
hir_id: hir::HirId,
span: Span,
) -> (Res, Option<RawTy<'tcx>>, &'tcx [hir::PathSegment<'tcx>]) {
) -> (Res, Option<Ty<'tcx>>, &'tcx [hir::PathSegment<'tcx>]) {
debug!(
"resolve_ty_and_res_fully_qualified_call: qpath={:?} hir_id={:?} span={:?}",
qpath, hir_id, span
Expand All @@ -841,23 +857,28 @@
// We manually call `register_wf_obligation` in the success path
// below.
let ty = self.astconv().ast_ty_to_ty_in_path(qself);
(self.handle_raw_ty(span, ty), qself, segment)
(ty, qself, segment)
}
QPath::LangItem(..) => {
bug!("`resolve_ty_and_res_fully_qualified_call` called on `LangItem`")
}
};

// FIXME(-Ztrait-solver=next): Can use `try_structurally_resolve` after migration.
let normalized =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let normalized =
let normalized_ty =

if !ty.is_ty_var() { self.structurally_normalize_after_astconv(span, ty) } else { ty };

if let Some(&cached_result) = self.typeck_results.borrow().type_dependent_defs().get(hir_id)
{
self.register_wf_obligation(ty.raw.into(), qself.span, traits::WellFormed(None));
self.register_wf_obligation(ty.into(), qself.span, traits::WellFormed(None));
// Return directly on cache hit. This is useful to avoid doubly reporting
// errors with default match binding modes. See #44614.
let def = cached_result.map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id));
return (def, Some(ty), slice::from_ref(&**item_segment));
}
let item_name = item_segment.ident;
let result = self
.resolve_fully_qualified_call(span, item_name, ty.normalized, qself.span, hir_id)
.resolve_fully_qualified_call(span, item_name, normalized, qself.span, hir_id)
.and_then(|r| {
// lint bare trait if the method is found in the trait
if span.edition().at_least_rust_2021() && let Some(mut diag) = self.tcx.sess.diagnostic().steal_diagnostic(qself.span, StashKey::TraitMissingMethod) {
Expand All @@ -876,14 +897,14 @@
};

let trait_missing_method =
matches!(error, method::MethodError::NoMatch(_)) && ty.normalized.is_trait();
matches!(error, method::MethodError::NoMatch(_)) && normalized.is_trait();
// If we have a path like `MyTrait::missing_method`, then don't register
// a WF obligation for `dyn MyTrait` when method lookup fails. Otherwise,
// register a WF obligation so that we can detect any additional
// errors in the self type.
if !trait_missing_method {
self.register_wf_obligation(
ty.raw.into(),
ty.into(),
qself.span,
traits::WellFormed(None),
);
Expand All @@ -902,7 +923,7 @@
if item_name.name != kw::Empty {
if let Some(mut e) = self.report_method_error(
span,
ty.normalized,
normalized,
item_name,
SelfSource::QPath(qself),
error,
Expand All @@ -918,7 +939,7 @@
});

if result.is_ok() {
self.register_wf_obligation(ty.raw.into(), qself.span, traits::WellFormed(None));
self.register_wf_obligation(ty.into(), qself.span, traits::WellFormed(None));
}

// Write back the new resolution.
Expand Down Expand Up @@ -1082,7 +1103,7 @@
pub fn instantiate_value_path(
&self,
segments: &[hir::PathSegment<'_>],
self_ty: Option<RawTy<'tcx>>,
self_ty: Option<Ty<'tcx>>,
res: Res,
span: Span,
hir_id: hir::HirId,
Expand All @@ -1093,7 +1114,7 @@
Res::Local(_) | Res::SelfCtor(_) => vec![],
Res::Def(kind, def_id) => self.astconv().def_ids_for_value_path_segments(
segments,
self_ty.map(|ty| ty.raw),
self_ty.map(|ty| ty),
kind,
def_id,
span,
Expand All @@ -1107,8 +1128,8 @@
Res::Def(DefKind::Ctor(CtorOf::Variant, _), _)
if let Some(self_ty) = self_ty =>
{
let adt_def = self_ty.normalized.ty_adt_def().unwrap();
user_self_ty = Some(UserSelfTy { impl_def_id: adt_def.did(), self_ty: self_ty.raw });
let adt_def = self.structurally_normalize_after_astconv(span, self_ty).ty_adt_def().unwrap();
user_self_ty = Some(UserSelfTy { impl_def_id: adt_def.did(), self_ty });
is_alias_variant_ctor = true;
}
Res::Def(DefKind::AssocFn | DefKind::AssocConst, def_id) => {
Expand All @@ -1127,7 +1148,7 @@
// inherent impl, we need to record the
// `T` for posterity (see `UserSelfTy` for
// details).
let self_ty = self_ty.expect("UFCS sugared assoc missing Self").raw;
let self_ty = self_ty.expect("UFCS sugared assoc missing Self");
user_self_ty = Some(UserSelfTy { impl_def_id: container_id, self_ty });
}
}
Expand Down Expand Up @@ -1206,9 +1227,9 @@
path_segs.last().is_some_and(|PathSeg(def_id, _)| tcx.generics_of(*def_id).has_self);

let (res, self_ctor_args) = if let Res::SelfCtor(impl_def_id) = res {
let ty =
self.handle_raw_ty(span, tcx.at(span).type_of(impl_def_id).instantiate_identity());
match ty.normalized.ty_adt_def() {
let ty = tcx.at(span).type_of(impl_def_id).instantiate_identity();
let normalized = self.structurally_normalize_after_astconv(span, ty);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let normalized = self.structurally_normalize_after_astconv(span, ty);
let normalized_ty = self.structurally_normalize_after_astconv(span, ty);

match normalized.ty_adt_def() {
Some(adt_def) if adt_def.has_ctor() => {
let (ctor_kind, ctor_def_id) = adt_def.non_enum_variant().ctor.unwrap();
// Check the visibility of the ctor.
Expand All @@ -1218,7 +1239,7 @@
.emit_err(CtorIsPrivate { span, def: tcx.def_path_str(adt_def.did()) });
}
let new_res = Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id);
let user_args = Self::user_args_for_adt(ty);
let user_args = Self::user_args_for_adt(ty, normalized);
user_self_ty = user_args.user_self_ty;
(new_res, Some(user_args.args))
}
Expand All @@ -1227,7 +1248,7 @@
span,
"the `Self` constructor can only be used with tuple or unit structs",
);
if let Some(adt_def) = ty.normalized.ty_adt_def() {
if let Some(adt_def) = normalized.ty_adt_def() {
match adt_def.adt_kind() {
AdtKind::Enum => {
err.help("did you mean to use one of the enum's variants?");
Expand Down Expand Up @@ -1299,7 +1320,7 @@
self.fcx.astconv().ast_region_to_region(lt, Some(param)).into()
}
(GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
self.fcx.to_ty(ty).raw.into()
self.fcx.to_ty(ty).into()
}
(GenericParamDefKind::Const { .. }, GenericArg::Const(ct)) => {
self.fcx.const_arg_to_const(&ct.value, param.def_id).into()
Expand Down Expand Up @@ -1367,7 +1388,7 @@
def_id,
&[],
has_self,
self_ty.map(|s| s.raw),
self_ty.map(|s| s),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self_ty.map(|s| s),
self_ty,

?

&arg_count,
&mut CreateCtorSubstsContext {
fcx: self,
Expand Down
44 changes: 24 additions & 20 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::method::MethodCallee;
use crate::TupleArgumentsFlag::*;
use crate::{errors, Expectation::*};
use crate::{
struct_span_err, BreakableCtxt, Diverges, Expectation, FnCtxt, Needs, RawTy, TupleArgumentsFlag,
struct_span_err, BreakableCtxt, Diverges, Expectation, FnCtxt, Needs, TupleArgumentsFlag,
};
use rustc_ast as ast;
use rustc_data_structures::fx::FxIndexSet;
Expand Down Expand Up @@ -1346,25 +1346,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Result<(&'tcx ty::VariantDef, Ty<'tcx>), ErrorGuaranteed> {
let path_span = qpath.span();
let (def, ty) = self.finish_resolving_struct_path(qpath, path_span, hir_id);
let normalized = self.structurally_normalize_after_astconv(path_span, ty);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let normalized = self.structurally_normalize_after_astconv(path_span, ty);
let normalized_ty = self.structurally_normalize_after_astconv(path_span, ty);


let variant = match def {
Res::Err => {
let guar =
self.tcx.sess.delay_span_bug(path_span, "`Res::Err` but no error emitted");
self.set_tainted_by_errors(guar);
return Err(guar);
}
Res::Def(DefKind::Variant, _) => match ty.normalized.ty_adt_def() {
Some(adt) => {
Some((adt.variant_of_res(def), adt.did(), Self::user_args_for_adt(ty)))
}
_ => bug!("unexpected type: {:?}", ty.normalized),
Res::Def(DefKind::Variant, _) => match normalized.ty_adt_def() {
Some(adt) => Some((
adt.variant_of_res(def),
adt.did(),
Self::user_args_for_adt(ty, normalized),
)),
_ => bug!("unexpected type: {:?}", normalized),
},
Res::Def(DefKind::Struct | DefKind::Union | DefKind::TyAlias | DefKind::AssocTy, _)
| Res::SelfTyParam { .. }
| Res::SelfTyAlias { .. } => match ty.normalized.ty_adt_def() {
Some(adt) if !adt.is_enum() => {
Some((adt.non_enum_variant(), adt.did(), Self::user_args_for_adt(ty)))
}
| Res::SelfTyAlias { .. } => match normalized.ty_adt_def() {
Some(adt) if !adt.is_enum() => Some((
adt.non_enum_variant(),
adt.did(),
Self::user_args_for_adt(ty, normalized),
)),
_ => None,
},
_ => bug!("unexpected definition: {:?}", def),
Expand All @@ -1379,9 +1385,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Check bounds on type arguments used in the path.
self.add_required_obligations_for_hir(path_span, did, args, hir_id);

Ok((variant, ty.normalized))
Ok((variant, normalized))
} else {
Err(match *ty.normalized.kind() {
Err(match *normalized.kind() {
ty::Error(guar) => {
// E0071 might be caused by a spelling error, which will have
// already caused an error message and probably a suggestion
Expand All @@ -1394,7 +1400,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
path_span,
E0071,
"expected struct, variant or union type, found {}",
ty.normalized.sort_string(self.tcx)
normalized.sort_string(self.tcx)
)
.span_label(path_span, "not a struct")
.emit(),
Expand Down Expand Up @@ -1802,23 +1808,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
qpath: &QPath<'_>,
path_span: Span,
hir_id: hir::HirId,
) -> (Res, RawTy<'tcx>) {
) -> (Res, Ty<'tcx>) {
match *qpath {
QPath::Resolved(ref maybe_qself, ref path) => {
let self_ty = maybe_qself.as_ref().map(|qself| self.to_ty(qself).raw);
let self_ty = maybe_qself.as_ref().map(|qself| self.to_ty(qself));
let ty = self.astconv().res_to_ty(self_ty, path, hir_id, true);
(path.res, self.handle_raw_ty(path_span, ty))
(path.res, ty)
}
QPath::TypeRelative(ref qself, ref segment) => {
let ty = self.to_ty(qself);

let result = self
.astconv()
.associated_path_to_ty(hir_id, path_span, ty.raw, qself, segment, true);
.associated_path_to_ty(hir_id, path_span, ty, qself, segment, true);
let ty = result
.map(|(ty, _, _)| ty)
.unwrap_or_else(|guar| Ty::new_error(self.tcx(), guar));
let ty = self.handle_raw_ty(path_span, ty);
let result = result.map(|(_, kind, def_id)| (kind, def_id));

// Write back the new resolution.
Expand All @@ -1827,8 +1832,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(result.map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id)), ty)
}
QPath::LangItem(lang_item, span, id) => {
let (res, ty) = self.resolve_lang_item_path(lang_item, span, hir_id, id);
(res, self.handle_raw_ty(path_span, ty))
self.resolve_lang_item_path(lang_item, span, hir_id, id)
}
}
}
Expand Down
12 changes: 0 additions & 12 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,15 +325,3 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
Some(&self.infcx)
}
}

/// Represents a user-provided type in the raw form (never normalized).
///
/// This is a bridge between the interface of `AstConv`, which outputs a raw `Ty`,
/// and the API in this module, which expect `Ty` to be fully normalized.
#[derive(Clone, Copy, Debug)]
pub struct RawTy<'tcx> {
pub raw: Ty<'tcx>,

/// The normalized form of `raw`, stored here for efficiency.
pub normalized: Ty<'tcx>,
}
Loading
Loading