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

Create a new TyKind::DynStar variant #107908

Closed
wants to merge 8 commits into from

Conversation

eholk
Copy link
Contributor

@eholk eholk commented Feb 10, 2023

Previously dyn* was implemented by adding a DynKind field on the ty::Dynamic variant. This works well for cases where we can treat the two types mostly the same, which is most of the front parts of the compiler (type checker, etc.). It doesn't work so well for the backends like Miri, since the representation of a dyn and dyn* are quite different. This PR splits the dyn* type into its own variant to make it more explicit how it is handled.

r? compiler-errors

@rustbot rustbot added 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 10, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@eholk
Copy link
Contributor Author

eholk commented Feb 10, 2023

This is almost certainly going to break in CI... I fixed what I could test locally, but unfortunately I haven't found a great way to test things like cranelift on my own machine.

@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@eholk eholk force-pushed the dyn-star-variant branch 2 times, most recently from 9267996 to 329895b Compare February 10, 2023 23:51
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@eholk eholk force-pushed the dyn-star-variant branch 2 times, most recently from fdb72fc to 1febfb2 Compare February 11, 2023 00:31
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking cargo_metadata v0.15.3
    Checking rustfix v0.6.1
    Checking clippy_lints v0.1.69 (/checkout/src/tools/clippy/clippy_lints)
    Checking rustc-workspace-hack v1.0.0 (/checkout/src/tools/rustc-workspace-hack)
error[E0004]: non-exhaustive patterns: `rustc_type_ir::sty::TyKind::DynStar(_, _)` not covered
     |
     |
1387 |         break match *ty.kind() {
     |                     ^^^^^^^^^^ pattern `rustc_type_ir::sty::TyKind::DynStar(_, _)` not covered
     |
note: `rustc_type_ir::sty::TyKind<TyCtxt<'_>>` defined here
    --> /checkout/compiler/rustc_type_ir/src/sty.rs:132:5
     |
50   | pub enum TyKind<I: Interner> {
...
...
132  |     DynStar(I::ListBinderExistentialPredicate, I::Region),
     |     ^^^^^^^ not covered
     = note: the matched value is of type `rustc_type_ir::sty::TyKind<TyCtxt<'_>>`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1428 ~             }
1428 ~             }
1429 ~             rustc_type_ir::sty::TyKind::DynStar(_, _) => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `clippy_lints` due to previous error
Build completed unsuccessfully in 0:02:50

@RalfJung
Copy link
Member

This will also cause tons of conflicts with #107728 :/

@compiler-errors
Copy link
Member

Needs fixing, and probably should wait until Ralf's PR is landed.

@rustbot author

@rustbot rustbot 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 Feb 13, 2023
@bors
Copy link
Contributor

bors commented Feb 14, 2023

☔ The latest upstream changes (presumably #108052) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@eholk
ping from triage - can you post your status on this PR?

This will also cause tons of conflicts with #107728 :/

this has merged

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@@ -1,19 +1,23 @@
//! Checks for usage of `&Vec[_]` and `&String`.

use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::diagnostics::{
Copy link
Member

Choose a reason for hiding this comment

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

This file looks as if it was just re-formatted. Please don't commit formatting changes to Clippy in the Rust repo.

@compiler-errors
Copy link
Member

Gonna close this since it's pretty stale, and probably just needs to be done from scratch.

@RalfJung
Copy link
Member

RalfJung commented May 8, 2023

it's pretty stale, and probably just needs to be done from scratch.

Where is that tracked?

@compiler-errors
Copy link
Member

@RalfJung, what do you mean?

@RalfJung
Copy link
Member

RalfJung commented May 8, 2023 via email

@compiler-errors
Copy link
Member

Ah, no, we currently do not have one. I'll leave a comment on the dyn* tracking issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants