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

mir: add used_generic_parameters_needs_subst #74717

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Jul 24, 2020

Fixes #74636.

This PR adds a used_generic_parameters_needs_subst helper function which checks whether a type needs substitution, but only for parameters that the unused_generic_params query considers used. This is used in the MIR interpreter to make the check for some pointer casts and for reflection intrinsics more precise.

I've opened this as a draft PR because this might not be the approach we want to fix this issue and we have to decide what to do about the reflection case.

r? @eddyb
cc @lcnr @wesleywiser

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Jul 31, 2020

@rust-highfive rust-highfive assigned oli-obk and unassigned eddyb Jul 31, 2020
@davidtwco davidtwco marked this pull request as ready for review July 31, 2020 15:03
This commit adds a `ensure_monomorphic_enough` utility function which
checks whether a type needs substitution, but only for parameters
that the `unused_generic_params` query considers used.

`ensure_monomorphic_enough` is then used throughout interpret where
`needs_subst` checks previously existed (in particular, for some
pointer casts and for reflection intrinsics more precise).

Signed-off-by: David Wood <david@davidtw.co>
@davidtwco davidtwco force-pushed the issue-74636-polymorphized-closures-inherited-params branch from 4d1d988 to 59e621c Compare July 31, 2020 15:37
@oli-obk
Copy link
Contributor

oli-obk commented Jul 31, 2020

cc @rust-lang/wg-const-eval

@bors r+

@bors
Copy link
Contributor

bors commented Jul 31, 2020

📌 Commit 59e621c 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 Jul 31, 2020
@bors
Copy link
Contributor

bors commented Aug 1, 2020

⌛ Testing commit 59e621c with merge 22e6099...

@bors
Copy link
Contributor

bors commented Aug 1, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 22e6099 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 1, 2020
@bors bors merged commit 22e6099 into rust-lang:master Aug 1, 2020
@davidtwco davidtwco deleted the issue-74636-polymorphized-closures-inherited-params branch August 3, 2020 09:31
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 5, 2020
…optimisations, r=lcnr

polymorphization: various improvements

This PR includes a handful of polymorphisation-related changes:

- @Mark-Simulacrum's suggestions [from this comment](rust-lang#74633 (comment)):
    - Use a `FiniteBitSet<u32>` over a `FiniteBitSet<u64>` as most functions won't have 64 generic parameters.
    - Don't encode polymorphisation results in metadata when every parameter is used (in this case, just invoking polymorphisation will probably be quicker).
- @lcnr's suggestion [from this comment](rust-lang#74717 (comment)).
    - Add an debug assertion in `ensure_monomorphic_enough` to make sure that polymorphisation did what we expect.

r? @lcnr
@davidtwco davidtwco mentioned this pull request Aug 9, 2020
2 tasks
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

bug with polymorphisation + closures that inherit unused parameters
7 participants