Forbid usage of hir Infer const/ty variants in ambiguous contexts

[BoxyUwU]

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::Infers 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

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

[rustbot]

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[BoxyUwU]

sry fmease
r? @ghost

[rustbot]

Failed to set assignee to ghost: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

[BoxyUwU]

@bors try @rust-timer queue

[bors]

⌛ Trying commit c19e8c3 with merge 397cf05...

[bors]

💔 Test failed - checks-actions

[BoxyUwU]

@bors try @rust-timer queue

[bors]

⌛ Trying commit d53cf04 with merge 9ce13d8...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9ce13d8 (9ce13d8853ff501c75672ba2b97a12ad6b00251e)

[rust-timer]

Finished benchmarking commit (9ce13d8): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -3.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 4.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.3%	[3.4%, 7.4%]	15
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.3%	[3.4%, 7.4%]	15

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 764.798s -> 764.043s (-0.10%)
Artifact size: 325.77 MiB -> 325.78 MiB (0.00%)

[BoxyUwU]

yippee

[BoxyUwU]

Rebased and pushed additional commits for the reviews so if you want to look at just the new stuff u can just look at the commits after Bless and add tests. Will squash them into the other commits and whatnot before merging

[BoxyUwU]

@bors try

[bors]

⌛ Trying commit d7b916b with merge ae258e1fcbee4a062f5f3d869f06004d7cea088e...

[bors]

☀️ Try build successful - checks-actions
Build commit: ae258e1 (ae258e1fcbee4a062f5f3d869f06004d7cea088e)

[BoxyUwU]

@craterbot check

[craterbot]

👌 Experiment pr-135272 created and queued.
🤖 Automatically detected try build ae258e1
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-135272 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-135272 is completed!
📊 21 regressed and 3 fixed (570539 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[BoxyUwU]

aww lol

[BoxyUwU]

this is not a new bug from this PR this is a pre-existing bug in generic_arg_infer. ast::TyKind::Infer is not the only ast ty that can be lowered to a hir::TyKind::Infer- a ast::TyKind::Paren wrapping a ast::TyKind::Infer also can be. e.g. let a: Vec<(_)> will ice under this PR as we attempt to create a hir::TyKind::<AmbigArg>::Infer for the GenericArg to Vec

all the regressions are due to an old version of the geo crate containing a let a: Vec<(_)>