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

Nuke the const_to_allocation query #53847

Closed
oli-obk opened this issue Aug 31, 2018 · 2 comments
Closed

Nuke the const_to_allocation query #53847

oli-obk opened this issue Aug 31, 2018 · 2 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Aug 31, 2018

This query is not doing anything anymore and only used in two locations.

Steps for resolving this issue:

  1. grep for const_to_allocation and remember where the two tcx.const_to_allocation calls are
  2. remove every mention of it
  3. grep for ConstToAllocation
  4. remove every mentioon of it
  5. start building (this takes some time before it hits your use sites, let the computer work in parallel to you)
  6. go back to the former call sites and replace them by pattern matching on the argument to extract the necessary data from the ByRef and panic if it's not ByRef.
  7. remove all the comments at the call sites talking about the query
  8. open a PR
  9. ping @RalfJung on the PR and watch his happiness meter go up
@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Aug 31, 2018
@RalfJung
Copy link
Member

go back to the former call sites and replace them by pattern matching on the argument to extract the necessary data from the ByRef and panic if it's not ByRef.

We might just have a small helper function in librustc_mir/const_eval.rs doing this extraction. Depends on how often it is used, I guess.

@csmoe csmoe mentioned this issue Aug 31, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this issue Sep 1, 2018
@kennytm
Rollup merge of rust-lang#53850 - csmoe:const_to_alloc, r=RalfJung
a0d0060 
Nuke the `const_to_allocation` query

Closes rust-lang#53847
r? @RalfJung
`./x.py check` works anyway, let's checkout tests from ci.
@RalfJung
Copy link
Member

RalfJung commented Sep 1, 2018

Happiness meter up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@RalfJung @oli-obk