From 7c8e281f735d02cddfd5c5ff350482a19a3d62c5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 26 Aug 2024 15:53:39 -0400 Subject: [PATCH] Flesh out some TODOs --- compiler/rustc_hir_analysis/messages.ftl | 2 +- compiler/rustc_hir_analysis/src/collect.rs | 1 + .../src/collect/resolve_bound_vars.rs | 48 +++++++++++++++---- compiler/rustc_hir_analysis/src/errors.rs | 3 +- .../src/hir_ty_lowering/bounds.rs | 43 ++++++++++++++--- .../src/hir_ty_lowering/mod.rs | 15 +++++- compiler/rustc_resolve/src/late.rs | 4 +- 7 files changed, 97 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index f5e8119cdca..262277aaa2a 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -37,7 +37,7 @@ hir_analysis_assoc_kind_mismatch = expected {$expected}, found {$got} hir_analysis_assoc_kind_mismatch_wrap_in_braces_sugg = consider adding braces here -hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the associated type of a trait with uninferred generic parameters +hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the associated {$what} of a trait with uninferred generic parameters .suggestion = use a fully qualified path with inferred lifetimes hir_analysis_associated_type_trait_uninferred_generic_params_multipart_suggestion = use a fully qualified path with explicit lifetimes diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index ac9976148e2..435a0e31b56 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -507,6 +507,7 @@ fn lower_assoc_ty( inferred_sugg, bound, mpart_sugg, + what: "type", }), ) } diff --git a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs index 7d8fb39710f..b241f8a5317 100644 --- a/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs +++ b/compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs @@ -1845,19 +1845,38 @@ fn uninsert_lifetime_on_error( assert_eq!(old_value, Some(bad_def)); } - // TODO: + // When we have a return type notation type in a where clause, like + // `where ::method(..): Send`, we need to introduce new bound + // vars to the existing where clause's binder, to represent the lifetimes + // elided by the return-type-notation syntax. + // + // For example, given + // ``` + // trait Foo { + // async fn x<'r, T>(); + // } + // ``` + // and a bound that looks like: + // `for<'a, 'b> >::x(): Other<'b>` + // this is going to expand to something like: + // `for<'a, 'b, 'r, T> >::x::<'r, T>::{opaque#0}: Other<'b>`. + // + // We handle this similarly for associated-type-bound style return-type-notation + // in `visit_segment_args`. fn try_append_return_type_notation_params( &mut self, hir_id: HirId, hir_ty: &'tcx hir::Ty<'tcx>, ) { let hir::TyKind::Path(qpath) = hir_ty.kind else { - // TODO: + // We only care about path types here. All other self types + // (including nesting the RTN type in another type) don't do + // anything. return; }; let (mut bound_vars, item_def_id, item_segment) = match qpath { - // TODO: + // If we have a fully qualified method, then we don't need to do any special lookup. hir::QPath::Resolved(_, path) if let [.., item_segment] = &path.segments[..] && item_segment.args.is_some_and(|args| { @@ -1873,23 +1892,32 @@ fn try_append_return_type_notation_params( (vec![], item_def_id, item_segment) } - // TODO: + // If we have a type-dependent path, then we do need to do some lookup. hir::QPath::TypeRelative(qself, item_segment) if item_segment.args.is_some_and(|args| { matches!(args.parenthesized, hir::GenericArgsParentheses::ReturnTypeNotation) }) => { + // First, ignore a qself that isn't a type or `Self` param. Those are the + // only ones that support `T::Assoc` anyways in HIR lowering. let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = qself.kind else { return; }; - match path.res { Res::Def(DefKind::TyParam, _) | Res::SelfTyParam { trait_: _ } => { + // Get the generics of this type's hir owner. This is *different* + // from the generics of the parameter's definition, since we want + // to be able to resolve an RTN path on a nested body (e.g. method + // inside an impl) using the where clauses on the method. let Some(generics) = self.tcx.hir_owner_node(hir_id.owner).generics() else { return; }; + // Look for the first bound that contains an associated type that + // matches the segment that we're looking for. We ignore any subsequent + // bounds since we'll be emitting a hard error in HIR lowering, so this + // is purely speculative. let one_bound = generics.predicates.iter().find_map(|predicate| { let hir::WherePredicate::BoundPredicate(predicate) = predicate else { return None; @@ -1927,7 +1955,9 @@ fn try_append_return_type_notation_params( _ => return, }; - // TODO: + // Append the early-bound vars on the function, and then the late-bound ones. + // We actually turn type parameters into higher-ranked types here, but we + // deny them later in HIR lowering. bound_vars.extend(self.tcx.generics_of(item_def_id).own_params.iter().map(|param| { match param.kind { ty::GenericParamDefKind::Lifetime => ty::BoundVariableKind::Region( @@ -1941,11 +1971,13 @@ fn try_append_return_type_notation_params( })); bound_vars.extend(self.tcx.fn_sig(item_def_id).instantiate_identity().bound_vars()); - // TODO: + // SUBTLE: Stash the old bound vars onto the *item segment* before appending + // the new bound vars. We do this because we need to know how many bound vars + // are present on the binder explicitly (i.e. not return-type-notation vars) + // to do bound var shifting correctly in HIR lowering. let existing_bound_vars = self.map.late_bound_vars.get_mut(&hir_id).unwrap(); let existing_bound_vars_saved = existing_bound_vars.clone(); existing_bound_vars.extend(bound_vars); - // TODO: subtle self.record_late_bound_vars(item_segment.hir_id, existing_bound_vars_saved); } } diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 11b6fedf1f9..f332080a71d 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -787,7 +787,8 @@ pub(crate) struct AssociatedTypeTraitUninferredGenericParams { pub inferred_sugg: Option, pub bound: String, #[subdiagnostic] - pub mpart_sugg: Option, + pub mpart_sugg: Option, + pub what: &'static str, } #[derive(Subdiagnostic)] diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 1aaacf8db32..033334bec8b 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -465,7 +465,8 @@ pub(super) fn lower_assoc_item_constraint( Ok(()) } - // TODO: + /// Lower a type, possibly specially handling the type if it's a return type notation + /// which we otherwise deny in other positions. pub fn lower_ty_maybe_return_type_notation(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> { let hir::TyKind::Path(qpath) = hir_ty.kind else { return self.lower_ty(hir_ty); @@ -482,14 +483,16 @@ pub fn lower_ty_maybe_return_type_notation(&self, hir_ty: &hir::Ty<'tcx>) -> Ty< ) }) => { + // We don't allow generics on the module segments. let _ = self.prohibit_generic_args(mod_segments.iter(), GenericsArgsErrExtend::None); let Res::Def(DefKind::AssocFn, item_def_id) = path.res else { - bug!(); + bug!("expected RTN to resolve to associated fn"); }; let trait_def_id = tcx.parent(item_def_id); + // Good error for `where Trait::method(..): Send`. let Some(self_ty) = opt_self_ty else { return self.error_missing_qpath_self_ty( trait_def_id, @@ -508,6 +511,18 @@ pub fn lower_ty_maybe_return_type_notation(&self, hir_ty: &hir::Ty<'tcx>) -> Ty< ty::BoundConstness::NotConst, ); + // SUBTLE: As noted at the end of `try_append_return_type_notation_params` + // in `resolve_bound_vars`, we stash the explicit bound vars of the where + // clause onto the item segment of the RTN type. This allows us to know + // how many bound vars are *not* coming from the signature of the function + // from lowering RTN itself. + // + // For example, in `where for<'a> >::method(..): Other`, + // the `late_bound_vars` of the where clause predicate (i.e. this HIR ty's + // parent) will include `'a` AND all the early- and late-bound vars of the + // method. But when lowering the RTN type, we just want the list of vars + // we used to resolve the trait ref. We explicitly stored those back onto + // the item segment, since there's no other good place to put them. let candidate = ty::Binder::bind_with_vars(trait_ref, tcx.late_bound_vars(item_segment.hir_id)); @@ -539,7 +554,8 @@ pub fn lower_ty_maybe_return_type_notation(&self, hir_ty: &hir::Ty<'tcx>) -> Ty< } } - // TODO: + /// Perform type-dependent lookup for a *method* for return type notation. + /// This generally mirrors `::lower_assoc_path`. fn resolve_type_relative_return_type_notation( &self, qself: &'tcx hir::Ty<'tcx>, @@ -592,12 +608,22 @@ fn resolve_type_relative_return_type_notation( _ => todo!(), }; - // Don't let `T::method` resolve to some `for<'a> >::method`. + // Don't let `T::method` resolve to some `for<'a> >::method`, + // which may happen via a higher-ranked where clause or supertrait. // This is the same restrictions as associated types; even though we could // support it, it just makes things a lot more difficult to support in - // `resolve_bound_vars`. + // `resolve_bound_vars`, since we'd need to introduce those as elided + // bound vars on the where clause too. if bound.has_bound_vars() { - todo!(); + return Err(self.tcx().dcx().emit_err( + errors::AssociatedItemTraitUninferredGenericParams { + span, + inferred_sugg: Some(span.with_hi(item_segment.ident.span.lo())), + bound: format!("{}::", tcx.anonymize_bound_vars(bound).skip_binder(),), + mpart_sugg: None, + what: "function", + }, + )); } let trait_def_id = bound.def_id(); @@ -608,7 +634,10 @@ fn resolve_type_relative_return_type_notation( Ok((bound, assoc_ty.def_id)) } - // TODO: + /// Do the common parts of lowering an RTN type. This involves extending the + /// candidate binder to include all of the early- and late-bound vars that are + /// defined on the function itself, and constructing a projection to the RPITIT + /// return type of that function. fn lower_return_type_notation_ty( &self, candidate: ty::PolyTraitRef<'tcx>, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index d42a70308d6..6998f580d80 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -2069,6 +2069,17 @@ pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> { }; self.lower_trait_object_ty(hir_ty.span, hir_ty.hir_id, bounds, lifetime, repr) } + // If we encounter a fully qualified path with RTN generics, then it must have + // *not* gone through `lower_ty_maybe_return_type_notation`, and therefore + // it's certainly in an illegal position. + hir::TyKind::Path(hir::QPath::Resolved(_, path)) + if path.segments.last().and_then(|segment| segment.args).is_some_and(|args| { + matches!(args.parenthesized, hir::GenericArgsParentheses::ReturnTypeNotation) + }) => + { + let guar = self.dcx().emit_err(BadReturnTypeNotation { span: hir_ty.span }); + Ty::new_error(tcx, guar) + } hir::TyKind::Path(hir::QPath::Resolved(maybe_qself, path)) => { debug!(?maybe_qself, ?path); let opt_self_ty = maybe_qself.as_ref().map(|qself| self.lower_ty(qself)); @@ -2093,7 +2104,9 @@ pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> { ref i => bug!("`impl Trait` pointed to non-opaque type?? {:#?}", i), } } - // TODO: + // If we encounter a type relative path with RTN generics, then it must have + // *not* gone through `lower_ty_maybe_return_type_notation`, and therefore + // it's certainly in an illegal position. hir::TyKind::Path(hir::QPath::TypeRelative(_, segment)) if segment.args.is_some_and(|args| { matches!(args.parenthesized, hir::GenericArgsParentheses::ReturnTypeNotation) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 813d569710a..f1d9028f88c 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -790,7 +790,9 @@ fn visit_ty(&mut self, ty: &'ast Ty) { TyKind::Path(qself, path) => { self.diag_metadata.current_type_path = Some(ty); - // TODO: + // If we have a path that ends with `(..)`, then it must be + // return type notation. Resolve that path in the *value* + // namespace. let source = if let Some(seg) = path.segments.last() && let Some(args) = &seg.args && matches!(**args, GenericArgs::ParenthesizedElided(..))