Skip to content
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

Add const generics support for pattern types #123689

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

spastorino
Copy link
Member

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 9, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 9, 2024

HIR ty lowering was modified

cc @fmease

@spastorino spastorino force-pushed the pattern_types_const_generics branch from 3bd2744 to f7b9e47 Compare April 9, 2024 18:49
res: Res::Def(DefKind::ConstParam, def_id), ..
},
)) => {
let ty = tcx.type_of(def_id).instantiate_identity();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let ty = tcx.type_of(def_id).instantiate_identity();
let ty = tcx.type_of(def_id).no_bound_vars().expect("const parameter types cannot be generic");

Copy link
Member

@fmease fmease Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we would factor out the logic common between this code path here and lower_const_param 🤔, not blocking though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slowly wondering if we should just use entirely new hir nodes so we can just use AnonConst like other type system consts

@spastorino spastorino force-pushed the pattern_types_const_generics branch from f7b9e47 to 30c546a Compare April 9, 2024 19:43
@oli-obk
Copy link
Contributor

oli-obk commented Apr 10, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 10, 2024

📌 Commit 30c546a has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 10, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#118391 (Add `REDUNDANT_LIFETIMES` lint to detect lifetimes which are semantically redundant)
 - rust-lang#123534 (Windows: set main thread name without re-encoding)
 - rust-lang#123659 (Add support to intrinsics fallback body)
 - rust-lang#123689 (Add const generics support for pattern types)
 - rust-lang#123701 (Only assert for child/parent projection compatibility AFTER checking that theyre coming from the same place)
 - rust-lang#123702 (Further cleanup cfgs in the UI test suite)
 - rust-lang#123706 (rustdoc: reduce per-page HTML overhead)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f7ae68 into rust-lang:master Apr 10, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
Rollup merge of rust-lang#123689 - spastorino:pattern_types_const_generics, r=oli-obk

Add const generics support for pattern types

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 11, 2024
…, r=compiler-errors

Call lower_const_param instead of duplicating the code

Follow up of rust-lang#123689

r? `@oli-obk`

I had this commit in my old branch that I had forgotten about, `@fmease` pointed about this in rust-lang#123689

I've left the branches that are not `Range` as do nothing as that's what we are currently doing but maybe we want to err or something.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2024
Rollup merge of rust-lang#123738 - spastorino:reuse-lower-const-param, r=compiler-errors

Call lower_const_param instead of duplicating the code

Follow up of rust-lang#123689

r? `@oli-obk`

I had this commit in my old branch that I had forgotten about, `@fmease` pointed about this in rust-lang#123689

I've left the branches that are not `Range` as do nothing as that's what we are currently doing but maybe we want to err or something.
&hir::Path {
res: Res::Def(DefKind::ConstParam, def_id), ..
},
)) => {
Copy link
Member

@fmease fmease Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we're missing a call to prohibit_generic_args here. Right now, we permit:

pattern_type!(u32 is START::<(), i32, 2>..=END::<_>)

where START and END are u32s like in the UI test.

While we could add this check, this just reinforces oli's point: #123689 (comment), Zulip discussion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this while reviewing an unrelated PR.

@oli-obk oli-obk deleted the pattern_types_const_generics branch April 12, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants