Skip to content

Commit

Permalink
Auto merge of #125915 - camelid:const-arg-refactor, r=BoxyUwU
Browse files Browse the repository at this point in the history
Represent type-level consts with new-and-improved `hir::ConstArg`

### Summary

This is a step toward `min_generic_const_exprs`. We now represent all const
generic arguments using an enum that differentiates between const *paths*
(temporarily just bare const params) and arbitrary anon consts that may perform
computations. This will enable us to cleanly implement the `min_generic_const_args`
plan of allowing the use of generics in paths used as const args, while
disallowing their use in arbitrary anon consts. Here is a summary of the salient
aspects of this change:

- Add `current_def_id_parent` to `LoweringContext`

  This is needed to track anon const parents properly once we implement
  `ConstArgKind::Path` (which requires moving anon const def-creation
  outside of `DefCollector`).

- Create `hir::ConstArgKind` enum with `Path` and `Anon` variants. Use it in the
  existing `hir::ConstArg` struct, replacing the previous `hir::AnonConst` field.

- Use `ConstArg` for all instances of const args. Specifically, use it instead
  of `AnonConst` for assoc item constraints, array lengths, and const param
  defaults.

- 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 rust-lang/rust#127009.

### Followup items post-merge

- Use `ConstArgKind::Path` for all const paths, not just const params.
- Fix (no github dont close this issue) #127009
- If a path in generic args doesn't resolve as a type, try to resolve as a const
  instead (do this in rustc_resolve). Then remove the special-casing from
  `rustc_ast_lowering`, so that all params will automatically be lowered as
  `ConstArgKind::Path`.
- (?) Consider making `const_evaluatable_unchecked` a hard error, or at least
  trying it in crater

r? `@BoxyUwU`
  • Loading branch information
bors committed Jul 19, 2024
2 parents b0209dc + f2c1265 commit 8051d07
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 31 deletions.
9 changes: 4 additions & 5 deletions clippy_lints/src/large_stack_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 3 additions & 3 deletions clippy_lints/src/trailing_empty_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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
{
Expand Down
28 changes: 21 additions & 7 deletions clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -270,6 +269,21 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
}
}

fn const_arg(&self, const_arg: &Binding<&ConstArg<'_>>) {
match const_arg.value.kind {
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({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 {
Expand Down Expand Up @@ -602,10 +616,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);
},
}
},
Expand Down
32 changes: 25 additions & 7 deletions clippy_utils/src/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -411,14 +411,25 @@ 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()),
_ => false,
}
}

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
},
}
}

fn eq_lifetime(left: &Lifetime, right: &Lifetime) -> bool {
left.res == right.res
}
Expand Down Expand Up @@ -1123,7 +1134,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),
}
}

Expand All @@ -1134,12 +1145,19 @@ 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::Path(path) => self.hash_qpath(path),
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()),
}
}
Expand Down
16 changes: 9 additions & 7 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -904,7 +904,8 @@ 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
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)
Expand Down Expand Up @@ -933,7 +934,8 @@ 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
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);
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/author/repeat.stdout
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/min_ident_chars.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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, const N: usize>([T; N]);
| ^

error: aborting due to 33 previous errors

0 comments on commit 8051d07

Please sign in to comment.