bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #135272 - BoxyUwU:generic_arg_infer_reliability_2, r=compiler-errors

Browse source 
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
This commit is contained in:
bors 2025-01-24 11:12:01 +00:00
commit 8231e8599e
119 changed files with 1056 additions and 669 deletions
Download patch file Download diff file Expand all files Collapse all files

6
compiler/rustc_lint/src/internal.rs
View file

@ -5,8 +5,8 @@ use rustc_ast as ast;
use rustc_hir::def::Res;
use rustc_hir::def_id::DefId;
use rustc_hir::{
BinOp, BinOpKind, Expr, ExprKind, GenericArg, HirId, Impl, Item, ItemKind, Node, Pat, PatKind,
Path, PathSegment, QPath, Ty, TyKind,
AmbigArg, BinOp, BinOpKind, Expr, ExprKind, GenericArg, HirId, Impl, Item, ItemKind, Node, Pat,
PatKind, Path, PathSegment, QPath, Ty, TyKind,
};
use rustc_middle::ty::{self, GenericArgsRef, Ty as MiddleTy};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -159,7 +159,7 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind {
}
}
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx Ty<'tcx>) {
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx Ty<'tcx, AmbigArg>) {
match &ty.kind {
TyKind::Path(QPath::Resolved(_, path)) => {
if lint_ty_kind_usage(cx, &path.res) {

9
compiler/rustc_lint/src/late.rs
View file

@ -8,9 +8,8 @@ use std::cell::Cell;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::join;
use rustc_hir as hir;
use rustc_hir::def_id::{LocalDefId, LocalModDefId};
use rustc_hir::{HirId, intravisit as hir_visit};
use rustc_hir::{self as hir, AmbigArg, HirId, intravisit as hir_visit};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::Session;
@ -214,15 +213,11 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
})
}
fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) {
fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx, AmbigArg>) {
lint_callback!(self, check_ty, t);
hir_visit::walk_ty(self, t);
}
fn visit_infer(&mut self, inf: &'tcx hir::InferArg) {
hir_visit::walk_inf(self, inf);
}
fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _: Span, n: HirId) {
if !self.context.only_module {
self.process_mod(m, n);

3
compiler/rustc_lint/src/lints.rs
View file

@ -9,6 +9,7 @@ use rustc_errors::{
};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::VisitorExt;
use rustc_hir::{self as hir, MissingLifetimeKind};
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::inhabitedness::InhabitedPredicate;
@ -293,7 +294,7 @@ impl<'a> LintDiagnostic<'a, ()> for BuiltinTypeAliasBounds<'_> {
// avoid doing throwaway work in case the lint ends up getting suppressed.
let mut collector = ShorthandAssocTyCollector { qselves: Vec::new() };
if let Some(ty) = self.ty {
hir::intravisit::Visitor::visit_ty(&mut collector, ty);
collector.visit_ty_unambig(ty);
}
let affect_object_lifetime_defaults = self

4
compiler/rustc_lint/src/non_local_def.rs
View file

@ -1,6 +1,6 @@
use rustc_errors::MultiSpan;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::intravisit::{self, Visitor, VisitorExt};
use rustc_hir::{Body, HirId, Item, ItemKind, Node, Path, TyKind};
use rustc_middle::ty::TyCtxt;
use rustc_session::{declare_lint, impl_lint_pass};
@ -126,7 +126,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
// 1. We collect all the `hir::Path` from the `Self` type and `Trait` ref
// of the `impl` definition
let mut collector = PathCollector { paths: Vec::new() };
collector.visit_ty(&impl_.self_ty);
collector.visit_ty_unambig(&impl_.self_ty);
if let Some(of_trait) = &impl_.of_trait {
collector.visit_trait_ref(of_trait);
}

4
compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs
View file

@ -1,4 +1,4 @@
use rustc_hir as hir;
use rustc_hir::{self as hir, AmbigArg};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::fold::BottomUpFolder;
@ -67,7 +67,7 @@ declare_lint! {
declare_lint_pass!(OpaqueHiddenInferredBound => [OPAQUE_HIDDEN_INFERRED_BOUND]);
impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound {
fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'tcx>) {
fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'tcx, AmbigArg>) {
let hir::TyKind::OpaqueDef(opaque) = &ty.kind else {
return;
};

5
compiler/rustc_lint/src/pass_by_value.rs
View file

@ -1,6 +1,5 @@
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::{GenericArg, PathSegment, QPath, TyKind};
use rustc_hir::{self as hir, AmbigArg, GenericArg, PathSegment, QPath, TyKind};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
@ -22,7 +21,7 @@ declare_tool_lint! {
declare_lint_pass!(PassByValue => [PASS_BY_VALUE]);
impl<'tcx> LateLintPass<'tcx> for PassByValue {
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx, AmbigArg>) {
match &ty.kind {
TyKind::Ref(_, hir::MutTy { ty: inner_ty, mutbl: hir::Mutability::Not }) => {
if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) {

2
compiler/rustc_lint/src/passes.rs
View file

@ -25,7 +25,7 @@ macro_rules! late_lint_methods {
fn check_pat(a: &'tcx rustc_hir::Pat<'tcx>);
fn check_expr(a: &'tcx rustc_hir::Expr<'tcx>);
fn check_expr_post(a: &'tcx rustc_hir::Expr<'tcx>);
fn check_ty(a: &'tcx rustc_hir::Ty<'tcx>);
fn check_ty(a: &'tcx rustc_hir::Ty<'tcx, rustc_hir::AmbigArg>);
fn check_generic_param(a: &'tcx rustc_hir::GenericParam<'tcx>);
fn check_generics(a: &'tcx rustc_hir::Generics<'tcx>);
fn check_poly_trait_ref(a: &'tcx rustc_hir::PolyTraitRef<'tcx>);

8
compiler/rustc_lint/src/traits.rs
View file

@ -1,4 +1,4 @@
use rustc_hir::{self as hir, LangItem};
use rustc_hir::{self as hir, AmbigArg, LangItem};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::sym;
@ -110,8 +110,10 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
}
}
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
let hir::TyKind::TraitObject(bounds, _lifetime, _syntax) = &ty.kind else { return };
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx, AmbigArg>) {
let hir::TyKind::TraitObject(bounds, _lifetime_and_syntax_pointer) = &ty.kind else {
return;
};
for bound in &bounds[..] {
let def_id = bound.trait_ref.trait_def_id();
if def_id.is_some_and(|def_id| cx.tcx.is_lang_item(def_id, LangItem::Drop)) {

7
compiler/rustc_lint/src/types.rs
View file

@ -4,7 +4,8 @@ use std::ops::ControlFlow;
use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, VariantIdx, Variants, WrappingRange};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::DiagMessage;
use rustc_hir::{Expr, ExprKind, LangItem};
use rustc_hir::intravisit::VisitorExt;
use rustc_hir::{AmbigArg, Expr, ExprKind, LangItem};
use rustc_middle::bug;
use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton};
use rustc_middle::ty::{
@ -1526,7 +1527,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
impl<'a, 'b, 'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'a, 'b, 'tcx> {
fn visit_ty(&mut self, ty: &'_ hir::Ty<'_>) {
fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) {
debug!(?ty);
if let hir::TyKind::BareFn(hir::BareFnTy { abi, .. }) = ty.kind
&& !self.visitor.is_internal_abi(*abi)
@ -1554,7 +1555,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
let mut visitor = FnPtrFinder { visitor: self, spans: Vec::new(), tys: Vec::new() };
ty.visit_with(&mut visitor);
hir::intravisit::Visitor::visit_ty(&mut visitor, hir_ty);
visitor.visit_ty_unambig(hir_ty);
iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect()
}