about summary refs log tree commit diff
path: root/src/librustdoc
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2025-01-24 11:12:01 +0000
committerbors <bors@rust-lang.org>2025-01-24 11:12:01 +0000
commit8231e8599e238ff4e717639bd68c6abb8579fe8d (patch)
treebd5801d2bc7eb09acfa728ae6b5d83fafc249a1f /src/librustdoc
parent061ee95ce197dc9b276fc5363eddbfc0ecc08584 (diff)
parentc58fe21cb919aedce5408f478c86b19e8e17bab2 (diff)
Auto merge of #135272 - BoxyUwU:generic_arg_infer_reliability_2, r=compiler-errors
Forbid usage of `hir` `Infer` const/ty variants in ambiguous contexts

The feature `generic_arg_infer` allows providing `_` as an argument to const generics in order to infer them. This introduces a syntactic ambiguity as to whether generic arguments are type or const arguments. In order to get around this we introduced a fourth `GenericArg` variant, `Infer` used to represent `_` as an argument to generic parameters when we don't know if its a type or a const argument.

This made hir visitors that care about `TyKind::Infer` or `ConstArgKind::Infer` very error prone as checking for `TyKind::Infer`s in  `visit_ty` would find *some* type infer arguments but not *all* of them as they would sometimes be lowered to `GenericArg::Infer` instead.

Additionally the `visit_infer` method would previously only visit `GenericArg::Infer` not *all* infers (e.g. `TyKind::Infer`), this made it very easy to override `visit_infer` and expect it to visit all infers when in reality it would only visit *some* infers.

---

This PR aims to fix those issues by making the `TyKind` and `ConstArgKind` types generic over whether the infer types/consts are represented by `Ty/ConstArgKind::Infer` or out of line (e.g. by a `GenericArg::Infer` or accessible by overiding `visit_infer`). We then make HIR Visitors convert all const args and types to the versions where infer vars are stored out of line and call `visit_infer` in cases where a `Ty`/`Const` would previously have had a `Ty/ConstArgKind::Infer` variant:

API Summary
```rust
enum AmbigArg {}

enum Ty/ConstArgKind<Unambig = ()> {
   ...
   Infer(Unambig),
}

impl Ty/ConstArg {
  fn try_as_ambig_ty/ct(self) -> Option<Ty/ConstArg<AmbigArg>>;
}
impl Ty/ConstArg<AmbigArg> {
  fn as_unambig_ty/ct(self) -> Ty/ConstArg;
}

enum InferKind {
  Ty(Ty),
  Const(ConstArg),
  Ambig(InferArg),
}

trait Visitor {
  ...
  fn visit_ty/const_arg(&mut self, Ty/ConstArg<AmbigArg>) -> Self::Result;
  fn visit_infer(&mut self, id: HirId, sp: Span, kind: InferKind) -> Self::Result;
}

// blanket impl'd, not meant to be overriden
trait VisitorExt {
  fn visit_ty/const_arg_unambig(&mut self, Ty/ConstArg) -> Self::Result;
}

fn walk_unambig_ty/const_arg(&mut V, Ty/ConstArg) -> Self::Result;
fn walk_ty/const_arg(&mut V, Ty/ConstArg<AmbigArg>) -> Self::Result;
```

The end result is that `visit_infer` visits *all* infer args and is also the *only* way to visit an infer arg, `visit_ty` and `visit_const_arg` can now no longer encounter a `Ty/ConstArgKind::Infer`. Representing this in the type system means that it is now very difficult to mess things up, either accessing `TyKind::Infer` "just works" and you won't miss *some* type infers- or it doesn't work and you have to look at `visit_infer` or some `GenericArg::Infer` which forces you to think about the full complexity involved.

Unfortunately there is no lint right now about explicitly matching on uninhabited variants, I can't find the context for why this is the case :woman_shrugging:

I'm not convinced the framing of un/ambig ty/consts is necessarily the right one but I'm not sure what would be better. I somewhat like calling them full/partial types based on the fact that `Ty<Partial>`/`Ty<Full>` directly specifies how many of the type kinds are actually represented compared to `Ty<Ambig>` which which leaves that to the reader to figure out based on the logical consequences of it the type being in an ambiguous position.

---

tool changes have been modified in their own commits for easier reviewing by anyone getting cc'd from subtree changes. I also attempted to split out "bug fixes arising from the refactoring" into their own commit so they arent lumped in with a big general refactor commit

Fixes #112110
Diffstat (limited to 'src/librustdoc')
-rw-r--r--src/librustdoc/clean/mod.rs23
-rw-r--r--src/librustdoc/visit_ast.rs9
2 files changed, 21 insertions, 11 deletions
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index fbd934bf718..7853e311a04 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -1747,9 +1747,9 @@ fn maybe_expand_private_type_alias<'tcx>(
     };
     let hir::ItemKind::TyAlias(ty, generics) = alias else { return None };
 
-    let provided_params = &path.segments.last().expect("segments were empty");
+    let final_seg = &path.segments.last().expect("segments were empty");
     let mut args = DefIdMap::default();
-    let generic_args = provided_params.args();
+    let generic_args = final_seg.args();
 
     let mut indices: hir::GenericParamCount = Default::default();
     for param in generics.params.iter() {
@@ -1781,7 +1781,7 @@ fn maybe_expand_private_type_alias<'tcx>(
                 let type_ = generic_args.args.iter().find_map(|arg| match arg {
                     hir::GenericArg::Type(ty) => {
                         if indices.types == j {
-                            return Some(*ty);
+                            return Some(ty.as_unambig_ty());
                         }
                         j += 1;
                         None
@@ -1843,10 +1843,13 @@ pub(crate) fn clean_ty<'tcx>(ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> T
             ImplTrait(ty.bounds.iter().filter_map(|x| clean_generic_bound(x, cx)).collect())
         }
         TyKind::Path(_) => clean_qpath(ty, cx),
-        TyKind::TraitObject(bounds, lifetime, _) => {
+        TyKind::TraitObject(bounds, lifetime) => {
             let bounds = bounds.iter().map(|bound| clean_poly_trait_ref(bound, cx)).collect();
-            let lifetime =
-                if !lifetime.is_elided() { Some(clean_lifetime(lifetime, cx)) } else { None };
+            let lifetime = if !lifetime.is_elided() {
+                Some(clean_lifetime(lifetime.pointer(), cx))
+            } else {
+                None
+            };
             DynTrait(bounds, lifetime)
         }
         TyKind::BareFn(barefn) => BareFunction(Box::new(clean_bare_fn_ty(barefn, cx))),
@@ -1854,7 +1857,7 @@ pub(crate) fn clean_ty<'tcx>(ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> T
             UnsafeBinder(Box::new(clean_unsafe_binder_ty(unsafe_binder_ty, cx)))
         }
         // Rustdoc handles `TyKind::Err`s by turning them into `Type::Infer`s.
-        TyKind::Infer
+        TyKind::Infer(())
         | TyKind::Err(_)
         | TyKind::Typeof(..)
         | TyKind::InferDelegation(..)
@@ -2533,8 +2536,10 @@ fn clean_generic_args<'tcx>(
                     GenericArg::Lifetime(clean_lifetime(lt, cx))
                 }
                 hir::GenericArg::Lifetime(_) => GenericArg::Lifetime(Lifetime::elided()),
-                hir::GenericArg::Type(ty) => GenericArg::Type(clean_ty(ty, cx)),
-                hir::GenericArg::Const(ct) => GenericArg::Const(Box::new(clean_const(ct, cx))),
+                hir::GenericArg::Type(ty) => GenericArg::Type(clean_ty(ty.as_unambig_ty(), cx)),
+                hir::GenericArg::Const(ct) => {
+                    GenericArg::Const(Box::new(clean_const(ct.as_unambig_ct(), cx)))
+                }
                 hir::GenericArg::Infer(_inf) => GenericArg::Infer,
             })
             .collect::<Vec<_>>()
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index d46b0dee36c..e4628e4f837 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -598,8 +598,13 @@ impl<'tcx> Visitor<'tcx> for RustdocVisitor<'_, 'tcx> {
         // Unneeded.
     }
 
-    fn visit_infer(&mut self, _: &hir::InferArg) {
-        // Unneeded.
+    fn visit_infer(
+        &mut self,
+        _inf_id: hir::HirId,
+        _inf_span: Span,
+        _kind: hir::intravisit::InferKind<'tcx>,
+    ) -> Self::Result {
+        // Unneeded
     }
 
     fn visit_lifetime(&mut self, _: &hir::Lifetime) {