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

Polymorphization: Look at called functions for usedness of generic params #106233

Closed
wants to merge 5 commits into from

Conversation

Nilstrieb
Copy link
Member

When determining whether a generic param is unused, we consider it unused even if it's used as a subst to a function const as long as it's not used within this function.

For example

fn unused<T>() {}
fn caller<T>() { unused::<T>() }

caller's T is considered unused now.

This causes a query cycle with recursive functions. I couldn't think of an easy way to avoid the cycle, so I just added non-fatal cycles to the query system. I don't know whether that's a good idea or a terrible abomination, cc @cjgillot on that.

It worked for a few quick tests but this needs some very careful review to make sure that we don't accidentally consider too many things as unused, which would be very bad.

@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2022

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 28, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

When a generic param is only used in a const fndef where it is in turn
unused, then it's unused in the caller as well.

This causes query cycles on recursive functions, we need to recover from
them and ignore them.
@Nilstrieb Nilstrieb 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 8, 2023
@cjgillot
Copy link
Contributor

cjgillot commented Jan 8, 2023

This causes a query cycle with recursive functions. I couldn't think of an easy way to avoid the cycle, so I just added non-fatal cycles to the query system. I don't know whether that's a good idea or a terrible abomination, cc @cjgillot on that.

The query system is not structured to allow such cycles. This is bound to cause ICEs.
Consider the following code:

fn foo<T, U>() { bar::<T>() }
fn bar<T>() { foo::<u8, T>() }

If we start by computing foo, we will have the following execution:

  • unused_generic_params(foo) calls unused_generic_params(bar).
    • unused_generic_params(bar) calls unused_generic_params(foo) -> cycle.
      • unused_generic_params(foo) cycle recovery returns [T: false, U: false].
    • unused_generic_params(bar) returns [T: false].
  • unused_generic_params(foo) returns [T: false, U: true].

If we start by computing bar, we will have the following execution:

  • unused_generic_params(bar) calls unused_generic_params(foo).
    • unused_generic_params(foo) calls unused_generic_params(bar) -> cycle.
      • unused_generic_params(bar) cycle recovery returns [T: false].
    • unused_generic_params(foo) returns [T: false, U: true].
  • unused_generic_params(bar) returns [T: true] --> different from previous execution.

So we have the query's value change depending on the order of query computation. This is unrelated to the contents of foo or bar, but may only depend on whether main calls foo or bar first. In this case, we'd have an "unstable fingerprint" ICE.

@cjgillot cjgillot self-assigned this Jan 8, 2023
@Nilstrieb
Copy link
Member Author

yeah that's what i thought and why i set it back to S-waiting-on-author, i need to find something better

Nilstrieb added a commit to Nilstrieb/rust that referenced this pull request Jan 11, 2023
…-errors

Polymorphization cleanup

Split out of rust-lang#106233

Use a newtype instead of a bitset directly. This makes the code way easier to read and easier to adapt for future changes.
Nilstrieb added a commit to Nilstrieb/rust that referenced this pull request Jan 11, 2023
…-errors

Polymorphization cleanup

Split out of rust-lang#106233

Use a newtype instead of a bitset directly. This makes the code way easier to read and easier to adapt for future changes.
@Nilstrieb
Copy link
Member Author

this needs a bunch of work if i continue working on this (which im not intending to do right now), so i'm closing this for now

@Nilstrieb Nilstrieb closed this Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants