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

Include Refs in Valtree Creation #95426

Merged
merged 6 commits into from
Apr 16, 2022
Merged

Include Refs in Valtree Creation #95426

merged 6 commits into from
Apr 16, 2022

Conversation

b-naber
Copy link
Contributor

@b-naber b-naber commented Mar 29, 2022 •
edited
Loading

This adds references to const_to_valtree, which isn't used in the compiler yet, but after the previous changes we made to the thir and mir representations and this change we should be able to finally introduce them in the next PR.

I wasn't able to properly test this code, except indirectly by including a call of const_to_valtree in the code that currently creates constants (turn_into_const_value).

r? @lcnr

cc @oli-obk @RalfJung

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 29, 2022
@rust-highfive
Copy link
Collaborator

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2022
@b-naber b-naber force-pushed the valtrees-slice branch 3 times, most recently from fd8b2ac to 95f8653 Compare April 8, 2022 13:42
@lcnr
Copy link
Contributor

lcnr commented Apr 13, 2022

I think this is mostly ready but won't be able to review this myself during the next 2 weeks

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned lcnr Apr 13, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Apr 13, 2022

@bors r+

Didn't do another full review, just an incremental from my last review

@bors
Copy link
Contributor

bors commented Apr 13, 2022

📌 Commit 4b126b8 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 Apr 13, 2022
@RalfJung
Copy link
Member

@bors r-

I am still confused, I wouldn't expect any types to be treated specially in the Ref match arm.

@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 Apr 13, 2022
@RalfJung
Copy link
Member

Also, is this code covered by any tests? Or is that not possible yet?

@b-naber
Copy link
Contributor Author

b-naber commented Apr 14, 2022

Also, is this code covered by any tests? Or is that not possible yet?

Don't think it's possible to test this yet, since we aren't calling this function anywhere yet.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 14, 2022

r? @RalfJung I'm going on vacation 😜

@rust-highfive rust-highfive assigned RalfJung and unassigned oli-obk Apr 14, 2022
@RalfJung
Copy link
Member

Don't think it's possible to test this yet, since we aren't calling this function anywhere yet.

Okay, that's a bummer. Not sure how I feel about landing dead code but I guess y'all have a plan. ;)

RalfJung
RalfJung reviewed Apr 16, 2022
View reviewed changes
compiler/rustc_const_eval/src/const_eval/mod.rs
const_to_valtree_inner(ecx, &derefd_place)
}
}
const_to_valtree_inner(ecx, &derefd_place)
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice, now the Ref case looks like I hoped it would. :)

@RalfJung
Copy link
Member

@bors r=RalfJung,oli-obk

@bors
Copy link
Contributor

bors commented Apr 16, 2022

📌 Commit d8205cd has been approved by RalfJung,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 Apr 16, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 16, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2022
@bors
Auto merge of rust-lang#96123 - Dylan-DPC:rollup-qjog6n1, r=Dylan-DPC
878c783 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#94985 (Parse inner attributes on inline const block)
 - rust-lang#95006 (Reject `#[thread_local]` attribute on non-static items)
 - rust-lang#95426 (Include Refs in Valtree Creation)
 - rust-lang#95908 (Inline `shallow_resolve_ty` into `ShallowResolver`)
 - rust-lang#96058 (separate flock implementations into separate modules)
 - rust-lang#96088 (Update mdbook)
 - rust-lang#96118 (rustdoc: Rename `def_id` into `item_id` when the type is `ItemId` for readability)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1dc672a into rust-lang:master Apr 16, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr left review comments

@oli-obk oli-obk oli-obk approved these changes

@RalfJung RalfJung RalfJung left review comments

Assignees

@RalfJung RalfJung

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
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.
7 participants
@b-naber @rust-highfive @lcnr @oli-obk @bors @RalfJung @rustbot