From 620a056a336971a4ca67d83fe472e4ca234bd51f Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sun, 2 Jun 2024 23:23:28 -0700 Subject: [PATCH 1/3] hir: Create `hir::ConstArgKind` enum This will allow lowering const params to a dedicated enum variant, rather than to an `AnonConst` that is later examined during `ty` lowering. --- clippy_utils/src/hir_utils.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 8706cec5d388..437f563cabd9 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -7,9 +7,9 @@ use rustc_data_structures::fx::FxHasher; use rustc_hir::def::Res; use rustc_hir::MatchSource::TryDesugar; use rustc_hir::{ - ArrayLen, AssocItemConstraint, BinOpKind, BindingMode, Block, BodyId, Closure, Expr, ExprField, ExprKind, FnRetTy, - GenericArg, GenericArgs, HirId, HirIdMap, InlineAsmOperand, LetExpr, Lifetime, LifetimeName, Pat, PatField, - PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, + ArrayLen, AssocItemConstraint, BinOpKind, BindingMode, Block, BodyId, Closure, ConstArg, ConstArgKind, Expr, + ExprField, ExprKind, FnRetTy, GenericArg, GenericArgs, HirId, HirIdMap, InlineAsmOperand, LetExpr, Lifetime, + LifetimeName, Pat, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; @@ -411,7 +411,7 @@ impl HirEqInterExpr<'_, '_, '_> { fn eq_generic_arg(&mut self, left: &GenericArg<'_>, right: &GenericArg<'_>) -> bool { match (left, right) { - (GenericArg::Const(l), GenericArg::Const(r)) => self.eq_body(l.value.body, r.value.body), + (GenericArg::Const(l), GenericArg::Const(r)) => self.eq_const_arg(l, r), (GenericArg::Lifetime(l_lt), GenericArg::Lifetime(r_lt)) => Self::eq_lifetime(l_lt, r_lt), (GenericArg::Type(l_ty), GenericArg::Type(r_ty)) => self.eq_ty(l_ty, r_ty), (GenericArg::Infer(l_inf), GenericArg::Infer(r_inf)) => self.eq_ty(&l_inf.to_ty(), &r_inf.to_ty()), @@ -419,6 +419,12 @@ impl HirEqInterExpr<'_, '_, '_> { } } + fn eq_const_arg(&mut self, left: &ConstArg<'_>, right: &ConstArg<'_>) -> bool { + match (&left.kind, &right.kind) { + (ConstArgKind::Anon(l_an), ConstArgKind::Anon(r_an)) => self.eq_body(l_an.body, r_an.body), + } + } + fn eq_lifetime(left: &Lifetime, right: &Lifetime) -> bool { left.res == right.res } @@ -1134,12 +1140,18 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.maybe_typeck_results = old_maybe_typeck_results; } + fn hash_const_arg(&mut self, const_arg: &ConstArg<'_>) { + match &const_arg.kind { + ConstArgKind::Anon(anon) => self.hash_body(anon.body), + } + } + fn hash_generic_args(&mut self, arg_list: &[GenericArg<'_>]) { for arg in arg_list { match *arg { GenericArg::Lifetime(l) => self.hash_lifetime(l), GenericArg::Type(ty) => self.hash_ty(ty), - GenericArg::Const(ref ca) => self.hash_body(ca.value.body), + GenericArg::Const(ref ca) => self.hash_const_arg(ca), GenericArg::Infer(ref inf) => self.hash_ty(&inf.to_ty()), } } From 6e99e2d748353174e23bf2849f0d523d8128a201 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 4 Jun 2024 18:24:08 -0700 Subject: [PATCH 2/3] Use `ConstArg` for array lengths --- clippy_lints/src/large_stack_arrays.rs | 9 ++++---- clippy_lints/src/trailing_empty_array.rs | 6 ++--- clippy_lints/src/utils/author.rs | 29 ++++++++++++++++++------ clippy_utils/src/hir_utils.rs | 4 ++-- clippy_utils/src/lib.rs | 18 +++++++++------ 5 files changed, 42 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs index c9bfc9c85d95..d94b0cce9486 100644 --- a/clippy_lints/src/large_stack_arrays.rs +++ b/clippy_lints/src/large_stack_arrays.rs @@ -106,13 +106,12 @@ fn might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { /// /// This is a fail-safe to a case where even the `is_from_proc_macro` is unable to determain the /// correct result. - fn repeat_expr_might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { - let ExprKind::Repeat(_, ArrayLen::Body(anon_const)) = expr.kind else { + fn repeat_expr_might_be_expanded<'tcx>(expr: &Expr<'tcx>) -> bool { + let ExprKind::Repeat(_, ArrayLen::Body(len_ct)) = expr.kind else { return false; }; - let len_span = cx.tcx.def_span(anon_const.def_id); - !expr.span.contains(len_span) + !expr.span.contains(len_ct.span()) } - expr.span.from_expansion() || is_from_proc_macro(cx, expr) || repeat_expr_might_be_expanded(cx, expr) + expr.span.from_expansion() || is_from_proc_macro(cx, expr) || repeat_expr_might_be_expanded(expr) } diff --git a/clippy_lints/src/trailing_empty_array.rs b/clippy_lints/src/trailing_empty_array.rs index 462084e96a86..db8c63892b84 100644 --- a/clippy_lints/src/trailing_empty_array.rs +++ b/clippy_lints/src/trailing_empty_array.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::has_repr_attr; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Const; +use rustc_middle::ty::{Const, FeedConstTy}; use rustc_session::declare_lint_pass; declare_clippy_lint! { @@ -53,14 +53,14 @@ impl<'tcx> LateLintPass<'tcx> for TrailingEmptyArray { } } -fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'_>, item: &Item<'_>) -> bool { +fn is_struct_with_trailing_zero_sized_array<'tcx>(cx: &LateContext<'tcx>, item: &Item<'tcx>) -> bool { if let ItemKind::Struct(data, _) = &item.kind // First check if last field is an array && let Some(last_field) = data.fields().last() && let rustc_hir::TyKind::Array(_, rustc_hir::ArrayLen::Body(length)) = last_field.ty.kind // Then check if that array is zero-sized - && let length = Const::from_anon_const(cx.tcx, length.def_id) + && let length = Const::from_const_arg(cx.tcx, length, FeedConstTy::No) && let length = length.try_eval_target_usize(cx.tcx, cx.param_env) && let Some(length) = length { diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index e9d69407df8b..441144d4bd11 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -5,10 +5,9 @@ use clippy_utils::{get_attr, higher}; use rustc_ast::ast::{LitFloatType, LitKind}; use rustc_ast::LitIntType; use rustc_data_structures::fx::FxHashMap; -use rustc_hir as hir; use rustc_hir::{ - ArrayLen, BindingMode, CaptureBy, Closure, ClosureKind, CoroutineKind, ExprKind, FnRetTy, HirId, Lit, PatKind, - QPath, StmtKind, TyKind, + self as hir, ArrayLen, BindingMode, CaptureBy, Closure, ClosureKind, ConstArg, ConstArgKind, CoroutineKind, + ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::declare_lint_pass; @@ -270,6 +269,22 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { } } + fn const_arg(&self, const_arg: &Binding<&ConstArg<'_>>) { + match const_arg.value.kind { + // FIXME: uncomment for ConstArgKind::Path + // ConstArgKind::Path(ref qpath) => { + // bind!(self, qpath); + // chain!(self, "let ConstArgKind::Path(ref {qpath}) = {const_arg}.kind"); + // self.qpath(qpath); + // }, + ConstArgKind::Anon(anon_const) => { + bind!(self, anon_const); + chain!(self, "let ConstArgKind::({anon_const}) = {const_arg}.kind"); + self.body(field!(anon_const.body)); + }, + } + } + fn lit(&self, lit: &Binding<&Lit>) { let kind = |kind| chain!(self, "let LitKind::{kind} = {lit}.node"); macro_rules! kind { @@ -602,10 +617,10 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { self.expr(value); match length.value { ArrayLen::Infer(..) => chain!(self, "let ArrayLen::Infer(..) = length"), - ArrayLen::Body(anon_const) => { - bind!(self, anon_const); - chain!(self, "let ArrayLen::Body({anon_const}) = {length}"); - self.body(field!(anon_const.body)); + ArrayLen::Body(const_arg) => { + bind!(self, const_arg); + chain!(self, "let ArrayLen::Body({const_arg}) = {length}"); + self.const_arg(const_arg); }, } }, diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 437f563cabd9..ae968067850e 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -227,7 +227,7 @@ impl HirEqInterExpr<'_, '_, '_> { pub fn eq_array_length(&mut self, left: ArrayLen<'_>, right: ArrayLen<'_>) -> bool { match (left, right) { (ArrayLen::Infer(..), ArrayLen::Infer(..)) => true, - (ArrayLen::Body(l_ct), ArrayLen::Body(r_ct)) => self.eq_body(l_ct.body, r_ct.body), + (ArrayLen::Body(l_ct), ArrayLen::Body(r_ct)) => self.eq_const_arg(l_ct, r_ct), (_, _) => false, } } @@ -1129,7 +1129,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { pub fn hash_array_length(&mut self, length: ArrayLen<'_>) { match length { ArrayLen::Infer(..) => {}, - ArrayLen::Body(anon_const) => self.hash_body(anon_const.body), + ArrayLen::Body(ct) => self.hash_const_arg(ct), } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index bdb3b5e45c48..f562d9787dc7 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -102,11 +102,11 @@ use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::{ - self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, ConstContext, - Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, - ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, OwnerNode, Param, Pat, - PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, - TyKind, UnOp, + self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, ConstArgKind, + ConstContext, Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, + ImplItemKind, ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, OwnerNode, + Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, + TraitRef, TyKind, UnOp, }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, Level, Lint, LintContext}; @@ -904,7 +904,9 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { }, ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default_equivalent(cx, x)), ExprKind::Repeat(x, ArrayLen::Body(len)) => { - if let ExprKind::Lit(const_lit) = cx.tcx.hir().body(len.body).value.kind + #[allow(irrefutable_let_patterns)] // FIXME + if let ConstArgKind::Anon(anon_const) = len.kind + && let ExprKind::Lit(const_lit) = cx.tcx.hir().body(anon_const.body).value.kind && let LitKind::Int(v, _) = const_lit.node && v <= 32 && is_default_equivalent(cx, x) @@ -933,7 +935,9 @@ fn is_default_equivalent_from(cx: &LateContext<'_>, from_func: &Expr<'_>, arg: & }) => return sym.is_empty() && is_path_lang_item(cx, ty, LangItem::String), ExprKind::Array([]) => return is_path_diagnostic_item(cx, ty, sym::Vec), ExprKind::Repeat(_, ArrayLen::Body(len)) => { - if let ExprKind::Lit(const_lit) = cx.tcx.hir().body(len.body).value.kind + #[allow(irrefutable_let_patterns)] // FIXME + if let ConstArgKind::Anon(anon_const) = len.kind + && let ExprKind::Lit(const_lit) = cx.tcx.hir().body(anon_const.body).value.kind && let LitKind::Int(v, _) = const_lit.node { return v == 0 && is_path_diagnostic_item(cx, ty, sym::Vec); From f2c1265483bcedb13cdac3b34432fbff7b4acd59 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 16 Jul 2024 19:07:36 -0700 Subject: [PATCH 3/3] Add `ConstArgKind::Path` and make `ConstArg` its own HIR node This is a very large commit since a lot needs to be changed in order to make the tests pass. The salient changes are: - `ConstArgKind` gets a new `Path` variant, and all const params are now represented using it. Non-param paths still use `ConstArgKind::Anon` to prevent this change from getting too large, but they will soon use the `Path` variant too. - `ConstArg` gets a distinct `hir_id` field and its own variant in `hir::Node`. This affected many parts of the compiler that expected the parent of an `AnonConst` to be the containing context (e.g., an array repeat expression). They have been changed to check the "grandparent" where necessary. - Some `ast::AnonConst`s now have their `DefId`s created in rustc_ast_lowering rather than `DefCollector`. This is because in some cases they will end up becoming a `ConstArgKind::Path` instead, which has no `DefId`. We have to solve this in a hacky way where we guess whether the `AnonConst` could end up as a path const since we can't know for sure until after name resolution (`N` could refer to a free const or a nullary struct). If it has no chance as being a const param, then we create a `DefId` in `DefCollector` -- otherwise we decide during ast_lowering. This will have to be updated once all path consts use `ConstArgKind::Path`. - We explicitly use `ConstArgHasType` for array lengths, rather than implicitly relying on anon const type feeding -- this is due to the addition of `ConstArgKind::Path`. - Some tests have their outputs changed, but the changes are for the most part minor (including removing duplicate or almost-duplicate errors). One test now ICEs, but it is for an incomplete, unstable feature and is now tracked at #127009. --- clippy_lints/src/utils/author.rs | 13 ++++++------- clippy_utils/src/hir_utils.rs | 6 ++++++ clippy_utils/src/lib.rs | 2 -- tests/ui/author/repeat.stdout | 3 ++- tests/ui/min_ident_chars.stderr | 8 +++++++- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 441144d4bd11..316c1f32d3a2 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -271,15 +271,14 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { fn const_arg(&self, const_arg: &Binding<&ConstArg<'_>>) { match const_arg.value.kind { - // FIXME: uncomment for ConstArgKind::Path - // ConstArgKind::Path(ref qpath) => { - // bind!(self, qpath); - // chain!(self, "let ConstArgKind::Path(ref {qpath}) = {const_arg}.kind"); - // self.qpath(qpath); - // }, + ConstArgKind::Path(ref qpath) => { + bind!(self, qpath); + chain!(self, "let ConstArgKind::Path(ref {qpath}) = {const_arg}.kind"); + self.qpath(qpath); + }, ConstArgKind::Anon(anon_const) => { bind!(self, anon_const); - chain!(self, "let ConstArgKind::({anon_const}) = {const_arg}.kind"); + chain!(self, "let ConstArgKind::Anon({anon_const}) = {const_arg}.kind"); self.body(field!(anon_const.body)); }, } diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index ae968067850e..6c6a237a8b16 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -421,7 +421,12 @@ impl HirEqInterExpr<'_, '_, '_> { fn eq_const_arg(&mut self, left: &ConstArg<'_>, right: &ConstArg<'_>) -> bool { match (&left.kind, &right.kind) { + (ConstArgKind::Path(l_p), ConstArgKind::Path(r_p)) => self.eq_qpath(l_p, r_p), (ConstArgKind::Anon(l_an), ConstArgKind::Anon(r_an)) => self.eq_body(l_an.body, r_an.body), + // Use explicit match for now since ConstArg is undergoing flux. + (ConstArgKind::Path(..), ConstArgKind::Anon(..)) | (ConstArgKind::Anon(..), ConstArgKind::Path(..)) => { + false + }, } } @@ -1142,6 +1147,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { fn hash_const_arg(&mut self, const_arg: &ConstArg<'_>) { match &const_arg.kind { + ConstArgKind::Path(path) => self.hash_qpath(path), ConstArgKind::Anon(anon) => self.hash_body(anon.body), } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index f562d9787dc7..8c33c34fa1c5 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -904,7 +904,6 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { }, ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default_equivalent(cx, x)), ExprKind::Repeat(x, ArrayLen::Body(len)) => { - #[allow(irrefutable_let_patterns)] // FIXME if let ConstArgKind::Anon(anon_const) = len.kind && let ExprKind::Lit(const_lit) = cx.tcx.hir().body(anon_const.body).value.kind && let LitKind::Int(v, _) = const_lit.node @@ -935,7 +934,6 @@ fn is_default_equivalent_from(cx: &LateContext<'_>, from_func: &Expr<'_>, arg: & }) => return sym.is_empty() && is_path_lang_item(cx, ty, LangItem::String), ExprKind::Array([]) => return is_path_diagnostic_item(cx, ty, sym::Vec), ExprKind::Repeat(_, ArrayLen::Body(len)) => { - #[allow(irrefutable_let_patterns)] // FIXME if let ConstArgKind::Anon(anon_const) = len.kind && let ExprKind::Lit(const_lit) = cx.tcx.hir().body(anon_const.body).value.kind && let LitKind::Int(v, _) = const_lit.node diff --git a/tests/ui/author/repeat.stdout b/tests/ui/author/repeat.stdout index c2a369610cc1..d9e3f864f12f 100644 --- a/tests/ui/author/repeat.stdout +++ b/tests/ui/author/repeat.stdout @@ -1,7 +1,8 @@ if let ExprKind::Repeat(value, length) = expr.kind && let ExprKind::Lit(ref lit) = value.kind && let LitKind::Int(1, LitIntType::Unsigned(UintTy::U8)) = lit.node - && let ArrayLen::Body(anon_const) = length + && let ArrayLen::Body(const_arg) = length + && let ConstArgKind::Anon(anon_const) = const_arg.kind && expr1 = &cx.tcx.hir().body(anon_const.body).value && let ExprKind::Lit(ref lit1) = expr1.kind && let LitKind::Int(5, LitIntType::Unsuffixed) = lit1.node diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr index 3dd5c9561fd0..7fbd8462fdcc 100644 --- a/tests/ui/min_ident_chars.stderr +++ b/tests/ui/min_ident_chars.stderr @@ -193,5 +193,11 @@ error: this ident consists of a single char LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { | ^ -error: aborting due to 32 previous errors +error: this ident consists of a single char + --> tests/ui/min_ident_chars.rs:93:41 + | +LL | struct Array([T; N]); + | ^ + +error: aborting due to 33 previous errors