-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
merge DefKind::InlineConst into AnonConst #158767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1635,29 +1635,32 @@ fn const_param_default<'tcx>( | |
| } | ||
|
|
||
| fn anon_const_kind<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> ty::AnonConstKind { | ||
| debug_assert_matches!(tcx.def_kind(def), DefKind::AnonConst | DefKind::InlineConst); | ||
| debug_assert_matches!(tcx.def_kind(def), DefKind::AnonConst); | ||
| let hir_id = tcx.local_def_id_to_hir_id(def); | ||
| let const_arg_id = tcx.parent_hir_id(hir_id); | ||
| match tcx.hir_node(const_arg_id) { | ||
| let parent_node_id = tcx.parent_hir_id(hir_id); | ||
| match tcx.hir_node(parent_node_id) { | ||
| hir::Node::ConstArg(const_arg) => { | ||
| debug_assert_matches!(const_arg.kind, hir::ConstArgKind::Anon(hir::AnonConst { def_id, .. }) if *def_id == def); | ||
| let parent_hir_node = tcx.hir_node(tcx.parent_hir_id(const_arg_id)); | ||
| if tcx.features().generic_const_exprs() { | ||
| ty::AnonConstKind::GCE | ||
| } else if tcx.features().min_generic_const_args() { | ||
| ty::AnonConstKind::MCG | ||
| } else if let hir::Node::Expr(hir::Expr { | ||
| kind: hir::ExprKind::Repeat(_, repeat_count), | ||
| .. | ||
| }) = parent_hir_node | ||
| && repeat_count.hir_id == const_arg_id | ||
| }) = tcx.parent_hir_node(parent_node_id) | ||
| && repeat_count.hir_id == parent_node_id | ||
| { | ||
| ty::AnonConstKind::RepeatExprCount | ||
| } else { | ||
| ty::AnonConstKind::MCG | ||
| } | ||
| } | ||
| _ => ty::AnonConstKind::NonTypeSystem, | ||
| hir::Node::Expr(hir::Expr { | ||
| kind: hir::ExprKind::ConstBlock(..) | hir::ExprKind::InlineAsm(..), | ||
| .. | ||
| }) => ty::AnonConstKind::NonTypeSystemInline, | ||
| _ => ty::AnonConstKind::NonTypeSystemAnon, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am unsure about this catch-all, tbh. There's three options I see:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it might make sense to make this |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -162,13 +162,17 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { | |
| ty::AnonConstKind::GCE => Some(parent_did), | ||
|
|
||
| // 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(_)) => | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm yeah I wouldn't expect pattern types to contain non type system anon consts? that feels weird :3 |
||
| { | ||
| Some(parent_did) | ||
| } | ||
| // Default to no generic parameters for other kinds of anon consts | ||
| ty::AnonConstKind::NonTypeSystem => None, | ||
| ty::AnonConstKind::NonTypeSystemAnon => None, | ||
| ty::AnonConstKind::NonTypeSystemInline => span_bug!( | ||
| tcx.def_span(def_id), | ||
| "a DefKind::AnonConst with a HIR parent of hir::Node::AnonConst should never be an AnonConstKind::NonTypeSystemInline" | ||
| ), | ||
| } | ||
| } | ||
| Node::ConstBlock(_) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,7 +190,7 @@ pub fn check_crate(tcx: TyCtxt<'_>) { | |
| // `feed_anon_const_type`. | ||
| // Also skip items for which typeck forwards to parent typeck. | ||
| if !(def_kind == DefKind::AnonConst | ||
| || def_kind == DefKind::InlineConst && tcx.is_type_system_inline_const(item_def_id) | ||
| && tcx.anon_const_kind(item_def_id) != ty::AnonConstKind::NonTypeSystemInline | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this now allows feeding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was previously allowed! This is one of the cases where my eyebrows were raised:
With this PR, before this PR, as well as before #158375 we blindly accepted all
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh that's quite fun, I would expect we never feed non type system anon consts lol. cc @oli-obk fine to keep behaviour the same for this PR |
||
| || tcx.is_typeck_child(item_def_id.to_def_id())) | ||
| { | ||
| tcx.ensure_ok().typeck(item_def_id); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might want to be more explicit that there are other kinds of anon consts too (e.g. enum discriminants)
View changes since the review