merge DefKind::InlineConst into AnonConst#158767
Conversation
|
This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 Some changes occurred to the CTFE machinery
cc @rust-lang/clippy Some changes occurred in cc @BoxyUwU HIR ty lowering was modified cc @fmease Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
S-waiting-on-review
rustbot
added
T-compiler
|
|
| kind: hir::ExprKind::ConstBlock(..) | hir::ExprKind::InlineAsm(..), | ||
| .. | ||
| }) => ty::AnonConstKind::NonTypeSystemInline, | ||
| _ => ty::AnonConstKind::NonTypeSystemAnon, |
There was a problem hiding this comment.
I am unsure about this catch-all, tbh. There's three options I see:
- leave it as a catch-all
- explicitly list which parents are allowed to have an AnonConst child, panic if there's an unexpected parent
- afaik it's just Node::Field and Node::Variant
- add variants to
AnonConstKindthat actually list which specific kind of anon const this non-type-system anon const is - for example, having the AnonConstKind variants ofConstBlock,AsmConstBlock,FieldAnonConst,VariantAnonConst, and then perhaps afn AnonConstKind::is_type_system() -> boolor somethin'- there's a few callers that we match on the hir parent again to figure out which type of non-type-system anon const it is, so just returning it here seems useful
| // Field defaults are allowed to use generic parameters, e.g. `field: u32 = /*defid: N + 1*/` | ||
| ty::AnonConstKind::NonTypeSystem | ||
| ty::AnonConstKind::NonTypeSystemAnon | ||
| if matches!(tcx.parent_hir_node(hir_id), Node::TyPat(_) | Node::Field(_)) => |
There was a problem hiding this comment.
Node::TyPat(_) appears to be dead code here - or at the very least, it's untested, replacing it with a panic and ./x test tests/ui does not fire. Was added in #136284 and code has been heavily refactored since then - haven't fully investigated yet.
| ExprKind::ConstBlock(constant) => { | ||
| for attr in &expr.attrs { | ||
| visit::walk_attribute(self, attr); | ||
| } | ||
|
|
||
| let def = self | ||
| .create_def(constant.id, None, DefKind::InlineConst, constant.value.span) | ||
| .def_id(); | ||
| // use specifically walk_anon_const, not walk_expr, to skip self.visit_anon_const | ||
| self.with_parent(def, |this| visit::walk_anon_const(this, constant)); | ||
| } |
There was a problem hiding this comment.
yippee, less def_collector jank!
3 participants
This is a closely related followup to #158375 (a condition of merging that PR was doing this as a followup)
This merge conflicts with #158617 ; prefer merging that one first please~
This PR is within the general goal of const generics /
generic_const_argsand perhaps specifically rust-lang/project-const-generics#108 but is mostly a code cleanup rather than implementing any particular featureAnon consts (the
1 + 2inlet x: [u8; 1 + 2];) are closely related to inline consts (theconst {}inlet x = const { 1 + 2 };). They are especially related now that both can be used in the type system (by the above PR) and should be treated identically in that scenario. In general, they should be treated the same, as evidenced by the number of matches touched in this PR that just match on both. This PR merges the two DefKinds into one, mostly arbitrarily and bikesheddily choosing to use the nameAnonConstfor the merged concept (e.g. bothDefKind::InlineConstandDefKind::AnonConstusedast::AnonConst, soast::AnonConstshould probably be named whatever the mergedDefKindconcept is named).When you absolute must distinguish between the two,
tcx.anon_const_kindallows you to do so - however, anon consts/inline consts in the type system intentionally appear identically underanon_const_kind. Indeed, the places where we distinguish between the two raised my eyebrows a few times when doing this PR, and being a little more explicit and loud about it is helpful for code clarity, IMO.r? @BoxyUwU