Skip to content

Commit 6180622

Browse files
committed
Auto merge of rust-lang#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 🤷‍♀️ 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 rust-lang#112110
2 parents d7b3940 + 3309f02 commit 6180622

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+168
-171
lines changed

clippy_lints/src/box_default.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ use clippy_utils::ty::expr_sig;
44
use clippy_utils::{is_default_equivalent, path_def_id};
55
use rustc_errors::Applicability;
66
use rustc_hir::def::Res;
7-
use rustc_hir::intravisit::{Visitor, walk_ty};
8-
use rustc_hir::{Block, Expr, ExprKind, LetStmt, Node, QPath, Ty, TyKind};
7+
use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_ty};
8+
use rustc_hir::{AmbigArg, Block, Expr, ExprKind, HirId, LetStmt, Node, QPath, Ty, TyKind};
99
use rustc_lint::{LateContext, LateLintPass, LintContext};
1010
use rustc_middle::lint::in_external_macro;
1111
use rustc_session::declare_lint_pass;
12-
use rustc_span::sym;
12+
use rustc_span::{Span, sym};
1313

1414
declare_clippy_lint! {
1515
/// ### What it does
@@ -92,8 +92,13 @@ fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>)
9292
struct InferVisitor(bool);
9393

9494
impl Visitor<'_> for InferVisitor {
95-
fn visit_ty(&mut self, t: &Ty<'_>) {
96-
self.0 |= matches!(t.kind, TyKind::Infer | TyKind::OpaqueDef(..) | TyKind::TraitObject(..));
95+
fn visit_infer(&mut self, inf_id: HirId, _inf_span: Span, _kind: InferKind<'_>) -> Self::Result {
96+
self.0 = true;
97+
self.visit_id(inf_id);
98+
}
99+
100+
fn visit_ty(&mut self, t: &Ty<'_, AmbigArg>) {
101+
self.0 |= matches!(t.kind, TyKind::OpaqueDef(..) | TyKind::TraitObject(..));
97102
if !self.0 {
98103
walk_ty(self, t);
99104
}
@@ -104,7 +109,7 @@ fn given_type(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
104109
match cx.tcx.parent_hir_node(expr.hir_id) {
105110
Node::LetStmt(LetStmt { ty: Some(ty), .. }) => {
106111
let mut v = InferVisitor::default();
107-
v.visit_ty(ty);
112+
v.visit_ty_unambig(ty);
108113
!v.0
109114
},
110115
Node::Expr(Expr {

clippy_lints/src/casts/as_pointer_underscore.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use rustc_middle::ty::Ty;
44

55
pub fn check<'tcx>(cx: &LateContext<'tcx>, ty_into: Ty<'_>, cast_to_hir: &'tcx rustc_hir::Ty<'tcx>) {
66
if let rustc_hir::TyKind::Ptr(rustc_hir::MutTy { ty, .. }) = cast_to_hir.kind
7-
&& matches!(ty.kind, rustc_hir::TyKind::Infer)
7+
&& matches!(ty.kind, rustc_hir::TyKind::Infer(()))
88
{
99
clippy_utils::diagnostics::span_lint_and_sugg(
1010
cx,

clippy_lints/src/casts/as_underscore.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_middle::ty;
77
use super::AS_UNDERSCORE;
88

99
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, ty: &'tcx Ty<'_>) {
10-
if matches!(ty.kind, TyKind::Infer) {
10+
if matches!(ty.kind, TyKind::Infer(())) {
1111
span_lint_and_then(cx, AS_UNDERSCORE, expr.span, "using `as _` conversion", |diag| {
1212
let ty_resolved = cx.typeck_results().expr_ty(expr);
1313
if let ty::Error(_) = ty_resolved.kind() {

clippy_lints/src/casts/cast_lossless.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub(super) fn check(
3838
return;
3939
};
4040
match cast_to_hir.kind {
41-
TyKind::Infer => {
41+
TyKind::Infer(()) => {
4242
diag.span_suggestion_verbose(
4343
expr.span,
4444
"use `Into::into` instead",

clippy_lints/src/casts/cast_ptr_alignment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
2424
&& let Some(generic_args) = method_path.args
2525
&& let [GenericArg::Type(cast_to)] = generic_args.args
2626
// There probably is no obvious reason to do this, just to be consistent with `as` cases.
27-
&& !is_hir_ty_cfg_dependant(cx, cast_to)
27+
&& !is_hir_ty_cfg_dependant(cx, cast_to.as_unambig_ty())
2828
{
2929
let (cast_from, cast_to) = (cx.typeck_results().expr_ty(self_arg), cx.typeck_results().expr_ty(expr));
3030
lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);

clippy_lints/src/casts/ptr_as_ptr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) {
4343
{
4444
let mut app = Applicability::MachineApplicable;
4545
let turbofish = match &cast_to_hir_ty.kind {
46-
TyKind::Infer => String::new(),
46+
TyKind::Infer(()) => String::new(),
4747
TyKind::Ptr(mut_ty) => {
48-
if matches!(mut_ty.ty.kind, TyKind::Infer) {
48+
if matches!(mut_ty.ty.kind, TyKind::Infer(())) {
4949
String::new()
5050
} else {
5151
format!(

clippy_lints/src/casts/ref_as_ptr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ pub(super) fn check<'tcx>(
3434

3535
let mut app = Applicability::MachineApplicable;
3636
let turbofish = match &cast_to_hir_ty.kind {
37-
TyKind::Infer => String::new(),
37+
TyKind::Infer(()) => String::new(),
3838
TyKind::Ptr(mut_ty) => {
39-
if matches!(mut_ty.ty.kind, TyKind::Infer) {
39+
if matches!(mut_ty.ty.kind, TyKind::Infer(())) {
4040
String::new()
4141
} else {
4242
format!(

clippy_lints/src/casts/unnecessary_cast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub(super) fn check<'tcx>(
4343
}
4444
},
4545
// Ignore `p as *const _`
46-
TyKind::Infer => return false,
46+
TyKind::Infer(()) => return false,
4747
_ => {},
4848
}
4949

clippy_lints/src/casts/zero_ptr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub fn check(cx: &LateContext<'_>, expr: &Expr<'_>, from: &Expr<'_>, to: &Ty<'_>
1818
Mutability::Not => ("`0 as *const _` detected", "ptr::null"),
1919
};
2020

21-
let sugg = if let TyKind::Infer = mut_ty.ty.kind {
21+
let sugg = if let TyKind::Infer(()) = mut_ty.ty.kind {
2222
format!("{std_or_core}::{sugg_fn}()")
2323
} else if let Some(mut_ty_snip) = mut_ty.ty.span.get_source_text(cx) {
2424
format!("{std_or_core}::{sugg_fn}::<{mut_ty_snip}>()")

clippy_lints/src/dereference.rs

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ use rustc_ast::util::parser::ExprPrecedence;
1111
use rustc_data_structures::fx::FxIndexMap;
1212
use rustc_errors::Applicability;
1313
use rustc_hir::def_id::DefId;
14-
use rustc_hir::intravisit::{Visitor, walk_ty};
14+
use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_ty};
1515
use rustc_hir::{
16-
self as hir, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node, Pat,
17-
PatKind, Path, QPath, TyKind, UnOp,
16+
self as hir, AmbigArg, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node,
17+
Pat, PatKind, Path, QPath, TyKind, UnOp,
1818
};
1919
use rustc_lint::{LateContext, LateLintPass};
2020
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
@@ -796,7 +796,7 @@ impl TyCoercionStability {
796796
if let Some(args) = path.args
797797
&& args.args.iter().any(|arg| match arg {
798798
hir::GenericArg::Infer(_) => true,
799-
hir::GenericArg::Type(ty) => ty_contains_infer(ty),
799+
hir::GenericArg::Type(ty) => ty_contains_infer(ty.as_unambig_ty()),
800800
_ => false,
801801
})
802802
{
@@ -815,7 +815,7 @@ impl TyCoercionStability {
815815
| TyKind::Path(_) => Self::Deref,
816816
TyKind::OpaqueDef(..)
817817
| TyKind::TraitAscription(..)
818-
| TyKind::Infer
818+
| TyKind::Infer(())
819819
| TyKind::Typeof(..)
820820
| TyKind::TraitObject(..)
821821
| TyKind::InferDelegation(..)
@@ -889,29 +889,23 @@ impl TyCoercionStability {
889889
fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
890890
struct V(bool);
891891
impl Visitor<'_> for V {
892-
fn visit_ty(&mut self, ty: &hir::Ty<'_>) {
893-
if self.0
894-
|| matches!(
895-
ty.kind,
896-
TyKind::OpaqueDef(..) | TyKind::Infer | TyKind::Typeof(_) | TyKind::Err(_)
897-
)
898-
{
892+
fn visit_infer(&mut self, inf_id: HirId, _inf_span: Span, kind: InferKind<'_>) -> Self::Result {
893+
if let InferKind::Ty(_) | InferKind::Ambig(_) = kind {
899894
self.0 = true;
900-
} else {
901-
walk_ty(self, ty);
902895
}
896+
self.visit_id(inf_id);
903897
}
904898

905-
fn visit_generic_arg(&mut self, arg: &hir::GenericArg<'_>) {
906-
if self.0 || matches!(arg, hir::GenericArg::Infer(_)) {
899+
fn visit_ty(&mut self, ty: &hir::Ty<'_, AmbigArg>) {
900+
if self.0 || matches!(ty.kind, TyKind::OpaqueDef(..) | TyKind::Typeof(_) | TyKind::Err(_)) {
907901
self.0 = true;
908-
} else if let hir::GenericArg::Type(ty) = arg {
909-
self.visit_ty(ty);
902+
} else {
903+
walk_ty(self, ty);
910904
}
911905
}
912906
}
913907
let mut v = V(false);
914-
v.visit_ty(ty);
908+
v.visit_ty_unambig(ty);
915909
v.0
916910
}
917911

0 commit comments

Comments
 (0)