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

extract ConstKind::Unevaluated into a struct #83040

Merged
merged 3 commits into from
Mar 21, 2021

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Mar 11, 2021

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 11, 2021
@rust-log-analyzer

This comment has been minimized.

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 think that after the valtree PR we should move promoteds out of ty::ConstKind::Unevaluated and directly as an option in mir::Constant, as that's the only place they can occur.

Comment on lines +590 to +535
(ty::ConstKind::Unevaluated(au), ty::ConstKind::Unevaluated(bu))
if tcx.features().const_evaluatable_checked && !relation.visit_ct_substs() =>
Copy link
Contributor

Choose a reason for hiding this comment

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

you lost the check for !promoted, though afaict you can just assert that actually

@@ -430,7 +430,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {

GenericArgKind::Const(constant) => {
match constant.val {
ty::ConstKind::Unevaluated(def, substs, promoted) => {
ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

PredicateKind::ConstEvaluatable could take a ty::Unevaluated, but considering that it can't take promoteds... maybe not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once val-tree is further ahead we should do that 👍

@lcnr lcnr force-pushed the unused-ct-substs branch from 3f802a6 to 2e18eb0 Compare March 13, 2021 15:31
@lcnr
Copy link
Contributor Author

lcnr commented Mar 13, 2021

rebased this on #83086 for now, as there are no merge conflicts there this might not actually be needed.

@rust-log-analyzer

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 13, 2021

as there are no merge conflicts there this might not actually be needed.

uh... I don't see how these two PRs relate except that both of them are a prerequisite for making substs optional ^^

@lcnr lcnr force-pushed the unused-ct-substs branch from 2e18eb0 to 05e16fc Compare March 13, 2021 16:12
@lcnr
Copy link
Contributor Author

lcnr commented Mar 13, 2021

removed that commit again 😆

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@lcnr lcnr force-pushed the unused-ct-substs branch from 36e749c to 78c87fc Compare March 13, 2021 17:40
@oli-obk
Copy link
Contributor

oli-obk commented Mar 14, 2021

This is still in draft mode, but seems fine to me

@lcnr lcnr marked this pull request as ready for review March 14, 2021 21:42
@lcnr
Copy link
Contributor Author

lcnr commented Mar 15, 2021

@oli-obk was that intended as an r=me?

I think it makes sense to merge this change as is.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 16, 2021

@bors r+

yea, should have been clearer on that :D

@bors
Copy link
Contributor

bors commented Mar 16, 2021

📌 Commit 78c87fc has been approved by oli-obk

@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 Mar 16, 2021
@bors
Copy link
Contributor

bors commented Mar 17, 2021

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

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 17, 2021
@lcnr lcnr force-pushed the unused-ct-substs branch from 78c87fc to 2885ca3 Compare March 20, 2021 16:25
@oli-obk
Copy link
Contributor

oli-obk commented Mar 20, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2021

📌 Commit 2885ca3 has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#82707 (const_evaluatable_checked: Stop eagerly erroring in `is_const_evaluatable`)
 - rust-lang#83040 (extract `ConstKind::Unevaluated` into a struct)
 - rust-lang#83280 (Fix pluralization in keyword docs)
 - rust-lang#83289 (Move some tests to more reasonable directories - 5)
 - rust-lang#83306 (Extend `proc_macro_back_compat` lint to `js-sys`)
 - rust-lang#83327 (Extend comment in `UsedLocals::visit_lhs`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 118aba3 into rust-lang:master Mar 21, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 21, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants