about summary refs log tree commit diff
path: root/compiler/rustc_trait_selection/src/errors.rs
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 /compiler/rustc_trait_selection/src/errors.rs
parent061ee95ce197dc9b276fc5363eddbfc0ecc08584 (diff)
parentc58fe21cb919aedce5408f478c86b19e8e17bab2 (diff)
downloadrust-8231e8599e238ff4e717639bd68c6abb8579fe8d.tar.gz
rust-8231e8599e238ff4e717639bd68c6abb8579fe8d.zip
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 'compiler/rustc_trait_selection/src/errors.rs')
-rw-r--r--compiler/rustc_trait_selection/src/errors.rs13
1 files changed, 6 insertions, 7 deletions
diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs
index 0bf91ad35c1..2dfa72972ba 100644
--- a/compiler/rustc_trait_selection/src/errors.rs
+++ b/compiler/rustc_trait_selection/src/errors.rs
@@ -6,11 +6,10 @@ use rustc_errors::{
     Applicability, Diag, DiagCtxtHandle, DiagMessage, DiagStyledString, Diagnostic,
     EmissionGuarantee, IntoDiagArg, Level, MultiSpan, SubdiagMessageOp, Subdiagnostic,
 };
-use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
-use rustc_hir::intravisit::{Visitor, walk_ty};
-use rustc_hir::{FnRetTy, GenericParamKind, Node};
+use rustc_hir::intravisit::{Visitor, VisitorExt, walk_ty};
+use rustc_hir::{self as hir, AmbigArg, FnRetTy, GenericParamKind, Node};
 use rustc_macros::{Diagnostic, Subdiagnostic};
 use rustc_middle::ty::print::{PrintTraitRefExt as _, TraitRefPrintOnlyTraitPath};
 use rustc_middle::ty::{self, Binder, ClosureKind, FnSig, PolyTraitRef, Region, Ty, TyCtxt};
@@ -579,7 +578,7 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
             }
 
             impl<'v> Visitor<'v> for ImplicitLifetimeFinder {
-                fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
+                fn visit_ty(&mut self, ty: &'v hir::Ty<'v, AmbigArg>) {
                     let make_suggestion = |ident: Ident| {
                         if ident.name == kw::Empty && ident.span.is_empty() {
                             format!("{}, ", self.suggestion_param_name)
@@ -642,16 +641,16 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
             if let Some(fn_decl) = node.fn_decl()
                 && let hir::FnRetTy::Return(ty) = fn_decl.output
             {
-                visitor.visit_ty(ty);
+                visitor.visit_ty_unambig(ty);
             }
             if visitor.suggestions.is_empty() {
                 // Do not suggest constraining the `&self` param, but rather the return type.
                 // If that is wrong (because it is not sufficient), a follow up error will tell the
                 // user to fix it. This way we lower the chances of *over* constraining, but still
                 // get the cake of "correctly" contrained in two steps.
-                visitor.visit_ty(self.ty_sup);
+                visitor.visit_ty_unambig(self.ty_sup);
             }
-            visitor.visit_ty(self.ty_sub);
+            visitor.visit_ty_unambig(self.ty_sub);
             if visitor.suggestions.is_empty() {
                 return false;
             }