Flesh out some TODOs

Author: compiler-errors

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

compiler/rustc_hir_analysis/src/collect.rs

@@ -507,6 +507,7 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
                     inferred_sugg,
                     bound,
                     mpart_sugg,
+                    what: "type",
                 }),
             )
         }

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

@@ -1862,19 +1862,38 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
         assert_eq!(old_value, Some(bad_def));
     }
 
-    // TODO:
+    // When we have a return type notation type in a where clause, like
+    // `where <T as Trait>::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> <T as Trait<'a>>::x(): Other<'b>`
+    // this is going to expand to something like:
+    //    `for<'a, 'b, 'r, T> <T as Trait<'a>>::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| {
@@ -1890,23 +1909,32 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
                 (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;
@@ -1944,7 +1972,9 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
             _ => 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(
@@ -1958,11 +1988,13 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
         }));
         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);
     }
 }

compiler/rustc_hir_analysis/src/errors.rs

@@ -787,7 +787,8 @@ pub(crate) struct AssociatedTypeTraitUninferredGenericParams {
     pub inferred_sugg: Option<Span>,
     pub bound: String,
     #[subdiagnostic]
-    pub mpart_sugg: Option<AssociatedTypeTraitUninferredGenericParamsMultipartSuggestion>,
+    pub mpart_sugg: Option<AssociatedItemTraitUninferredGenericParamsMultipartSuggestion>,
+    pub what: &'static str,
 }
 
 #[derive(Subdiagnostic)]

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

@@ -464,7 +464,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         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);
@@ -481,14 +482,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                         )
                     }) =>
             {
+                // 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,
@@ -507,6 +510,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                     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> <T as Trait<'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));
 
@@ -538,7 +553,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         }
     }
 
-    // TODO:
+    /// Perform type-dependent lookup for a *method* for return type notation.
+    /// This generally mirrors `<dyn HirTyLowerer>::lower_assoc_path`.
     fn resolve_type_relative_return_type_notation(
         &self,
         qself: &'tcx hir::Ty<'tcx>,
@@ -591,12 +607,22 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             _ => todo!(),
         };
 
-        // Don't let `T::method` resolve to some `for<'a> <T as Tr<'a>>::method`.
+        // Don't let `T::method` resolve to some `for<'a> <T as Tr<'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();
@@ -607,7 +633,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         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>,

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -2070,6 +2070,17 @@ impl<'tcx> dyn HirTyLowerer<'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));
@@ -2094,7 +2105,9 @@ impl<'tcx> dyn HirTyLowerer<'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)

compiler/rustc_resolve/src/late.rs

@@ -772,7 +772,9 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
             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(..))