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

CTFE: remove memoization leftovers #79667

Closed
RalfJung opened this issue Dec 3, 2020 · 3 comments · Fixed by #79840
Closed

CTFE: remove memoization leftovers #79667

RalfJung opened this issue Dec 3, 2020 · 3 comments · Fixed by #79840
Assignees
Labels
A-const-eval Area: constant evaluation (mir interpretation) 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

@RalfJung
Copy link
Member

RalfJung commented Dec 3, 2020

Since #79594, memoization of argumentless functions is restricted to intrinsics, because other functions could perform heap allocations and thus memoization would be incorrect.

I doubt we have any argumentless intrinsic that is expensive enough that memoization is worth it, so I propose we remove the leftovers of that infrastructure.

Cc @rust-lang/wg-const-eval

@RalfJung RalfJung added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Dec 3, 2020
@camelid camelid added the A-const-eval Area: constant evaluation (mir interpretation) label Dec 3, 2020
@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Dec 4, 2020
@dvtkrlbs
Copy link
Contributor

dvtkrlbs commented Dec 6, 2020

I want to work on this but i never contributed to rust-lang before so i need some guidance.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 7, 2020

Great! So... step 0 is to write @rustbot claim as a comment here, this will make sure the issue is assigned to you and noone else tries to tackle it at the same time.

The main change is to remove this function:

fn try_eval_const_fn_call(
entirely without a replacement. You'll then have to adjust the call site to act as if the function returned Ok(false). This may allow some small cleanups at the call site; I expect nothing major to happen there.

I do not think there will be further test changes due to this, so, ./x.py test src/test/ui should run through without any failures. If it doesn't, still open a PR and let us know of your findings.

@dvtkrlbs
Copy link
Contributor

dvtkrlbs commented Dec 7, 2020

@rustbot claim

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 15, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 15, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 15, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 16, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 16, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 16, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 17, 2020
Remove memoization leftovers from constant evaluation machine

Closes rust-lang#79667
@bors bors closed this as completed in 7cb74ed Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: constant evaluation (mir interpretation) 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
Development

Successfully merging a pull request may close this issue.

4 participants