Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #135272 - BoxyUwU:generic_arg_infer_reliability_2, r=co…
…mpiler-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 🤷♀️ 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
- Loading branch information