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.

Sign up for GitHub

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

Jump to bottom

Move ty::ConstKind to rustc_type_ir #113321

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Move ty::ConstKind to rustc_type_ir #113321

merged 2 commits into from
Jul 5, 2023

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Jul 4, 2023 •
edited
Loading

Needed this in another PR for custom debug impls, and this will also be required to move the new solver into a separate crate that does not use TyCtxt so that r-a and friends can depend on the trait solver.

Rebased on top of #113325, only the second and third commits needs reviewing

@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jul 4, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2023

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 4, 2023

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned petrochenkov Jul 4, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
oli-obk
oli-obk requested changes Jul 4, 2023
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

I guess I should not have reviewed commit by commit.

Looks ok modulo the mk_const removal.

Also probably requires clippy and miri changes

compiler/rustc_middle/src/infer/canonical.rs Outdated Show resolved Hide resolved
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 4, 2023

I will open a second PR to handle the construction of Const differently according to that mcp to have the methods on Const since it seems like needless churn to have everthing change to various tcx.mk_x and then Const::mk_x

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@BoxyUwU BoxyUwU force-pushed the move_constkind_to_typeir branch 2 times, most recently from af3396a to 308ee8a Compare July 4, 2023 14:42
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Jul 4, 2023

Separated out the construction changes to #113325 now the only changes in this PR are ones relevent to moving ConstKind to type ir, these are in the second and third commits. This PR is rebased on top of #113325 though since the existing ways of constructing Const wont work with it in type ir because the From impls don't exist anymore.

@bors
Copy link
Contributor

bors commented Jul 5, 2023

☔ The latest upstream changes (presumably #113325) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk
oli-obk reviewed Jul 5, 2023
View reviewed changes
compiler/rustc_middle/src/ty/structural_impls.rs
Comment on lines +714 to +725
let kind = match self.kind() {
ConstKind::Param(p) => ConstKind::Param(p.try_fold_with(folder)?),
ConstKind::Infer(i) => ConstKind::Infer(i.try_fold_with(folder)?),
ConstKind::Bound(d, b) => {
ConstKind::Bound(d.try_fold_with(folder)?, b.try_fold_with(folder)?)
}
ConstKind::Placeholder(p) => ConstKind::Placeholder(p.try_fold_with(folder)?),
ConstKind::Unevaluated(uv) => ConstKind::Unevaluated(uv.try_fold_with(folder)?),
ConstKind::Value(v) => ConstKind::Value(v.try_fold_with(folder)?),
ConstKind::Error(e) => ConstKind::Error(e.try_fold_with(folder)?),
ConstKind::Expr(e) => ConstKind::Expr(e.try_fold_with(folder)?),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

why isn't this an impl on the generic ConstKind with the appropriate bounds?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copied how the typefoldable stuff works for Ty. which is that we have the impls on Ty and TyKind isnt TypeVisitable or TypeFoldable. It's a change from the status quo but it doesn't really seem like we acutally used ConstKind: TypeVisitable/TypeFoldable anywhere except the Const: TypeVisitable/TypeFoldable impl and I rather have Const work the same as Ty where possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspected as much, makes sense, we can revisit if more usages ever come up

compiler/rustc_middle/src/ty/structural_impls.rs Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Jul 5, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jul 5, 2023

📌 Commit 62174bf 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 Jul 5, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 5, 2023
@Dylan-DPC
Rollup merge of rust-lang#113321 - BoxyUwU:move_constkind_to_typeir, …
f106202 
…r=oli-obk

Move `ty::ConstKind` to `rustc_type_ir`

Needed this in another PR for custom debug impls, and this will also be required to move the new solver into a separate crate that does not use `TyCtxt` so that r-a and friends can depend on the trait solver.

Rebased on top of rust-lang#113325, only the second and third commits needs reviewing
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 5, 2023
Closed
@compiler-errors compiler-errors mentioned this pull request Jul 5, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 5, 2023
@bors
Auto merge of rust-lang#113370 - compiler-errors:rollup-8gvyy8e, r=co…
5dac6b3 
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#113010 (rust-installer & rls: remove exclusion from rustfmt & tidy )
 - rust-lang#113317 ( -Ztrait-solver=next: stop depending on old solver)
 - rust-lang#113319 (`TypeParameterDefinition` always require a `DefId`)
 - rust-lang#113320 (Add some extra information to opaque type cycle errors)
 - rust-lang#113321 (Move `ty::ConstKind` to `rustc_type_ir`)
 - rust-lang#113337 (Winnow specialized impls during selection in new solver)
 - rust-lang#113355 (Move most coverage code out of `rustc_codegen_ssa`)
 - rust-lang#113356 (Add support for NetBSD/riscv64 aka. riscv64gc-unknown-netbsd.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b2b1a50 into rust-lang:master Jul 5, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jul 5, 2023
tshepang added a commit to tshepang/rustc-dev-guide that referenced this pull request Jul 7, 2023
@tshepang
ty::ConstKind has moved
9934e1b 
See rust-lang/rust#113321
@tshepang tshepang mentioned this pull request Jul 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.
6 participants
@BoxyUwU @rustbot @rust-log-analyzer @bors @oli-obk @petrochenkov