The 0.9 Windows installer installs empty dll files #11435
Comments
|
cc @brson |
|
The install is still functional, I assume? |
|
Nominating. |
|
accepted for P-low |
|
(tagging as E-easy in the hopes that it is easy for someone who knows their way around windows installers) |
|
It seems fixed: |
|
@klutzy: They're also gone from the |
|
Right. |
flip1995
pushed a commit
to flip1995/rust
that referenced
this issue
Sep 7, 2023
[`implied_bounds_in_impls`]: don't ICE on default generic parameter and move to nursery Fixes rust-lang#11422 This fixes two ICEs ([1](rust-lang/rust-clippy#11422 (comment)), [2](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2901e6febb479d3bd2a74f8a5b8a9305)), and moves it to nursery for now, because this lint needs some improvements in its suggestion (see rust-lang#11435, for one such example). changelog: Moved [`implied_bounds_in_impls`] to nursery (Now allow-by-default) [rust-lang#11437](rust-lang/rust-clippy#11437) changelog: [`implied_bounds_in_impls`]: don't ICE on default generic parameter in supertrait clause r? `@xFrednet` (since you reviewed my PR that added this lint, I figured it might make sense to have you review this as well since you have seen this code before. If you don't want to review this, sorry! Feel free to reroll then) -------- As for the ICE, it's pretty complicated and very confusing imo, so I'm going to try to explain the idea here (partly for myself, too, because I've confused myself several times writing- and fixing this): <details> <summary>Expand</summary> The general idea behind the lint is that, if we have this function: ```rs fn f() -> impl PartialEq<i32> + PartialOrd<i32> { 0 } ``` We want to lint the `PartialEq` bound because it's unnecessary. That exact bound is already specified in `PartialOrd<i32>`'s supertrait clause: ```rs trait PartialOrd<Rhs>: PartialEq<Rhs> {} // PartialOrd<i32>: PartialEq<i32> ``` The way it does this is in two steps: - Go through all of the bounds in the `impl Trait` return type and collect each of the trait's supertrait bounds into a vec. We also store the generic arguments for later. - `PartialEq` has no supertraits, nothing to add. - `PartialOrd` is defined as `trait PartialOrd: PartialEq`, so add `PartialEq` to the list, as well as the generic argument(s) `<i32>` Once we are done, we have these entries in the vec: `[(PartialEq, [i32])]` - Go through all the bounds again, and looking for those bounds that have their trait `DefId` in the implied bounds vec. - `PartialEq` is in that vec. However, that is not enough, because the trait is generic. If the user wrote `impl PartialEq<String> + PartialOrd<i32>`, then `PartialOrd` clearly doesn't imply `PartialEq`. Which means, we also need to check that the generic parameters match. This is why we also collected the generic arguments in `PartialOrd<i32>`. This process of checking generic arguments is pretty complicated and is also where the two ICEs happened. The way it checks that the generic arguments match is by comparing the generic parameters in the super trait clause: ```rs trait PartialOrd<Rhs>: PartialEq<Rhs> {} // ^^^^^^^^^^^^^^ ``` ...this needs to match... ```rs fn f() -> impl PartialEq<i32> + ... // ^^^^^^^^^^^^^^ ``` In the compiler, the `Rhs` generic parameter is its own type and we cannot just compare it to `i32`. We need to "substitute" it. Internally, `Rhs` is represented as `Rhs#1` (the number next to # represents the type parameter index. They start at 0, but 0 is "reserved" for the implicit `Self` generic parameter). How do we go from `Rhs#1` to `i32`? Well, we know that all the generic parameters had to be substituted in the `impl ... + PartialOrd<i32>` type. So we subtract 1 from the type parameter index, giving us 0 (`Self` is not specified in that list of arguments). We use that as the index into the generic argument list `<i32>`. That's `i32`. Now we know that the supertrait clause looks like `: PartialEq<i32>`. Then, we can compare that to what the user actually wrote on the bound that we think is being implied: `impl PartialEq<i32> + ...`. Now to the actual bug: this whole logic doesn't take into account *default* generic parameters. Actually, `PartialOrd` is defined like this: ```rs trait PartialOrd<Rhs = Self>: PartialEq<Rhs> {} ``` If we now have a function like this: ```rs fn f() -> impl PartialOrd + PartialEq {} ``` that logic breaks apart... We look at the supertrait predicate `: PartialEq<Rhs>` (`Rhs` is `Rhs#1`), then take the first argument in the generic argument list `PartialEq<..>` to resolve the `Rhs`, but at this point we crash because there *is no* generic argument. The index 0 is out of bounds. If this happens (and we even get to linting here, which could only happen if it passes typeck), it must mean that that generic parameter has a default type that is not required to be specified. This PR changes the logic such that if we have a type parameter index that is out of bounds, it looks at the definition of the trait and check that there exists a default type that we can use instead. So, we see `<Rhs = Self>`, and use `Self` for substitution, and end up with this predicate: `: PartialEq<Self>`. No crash this time. </details>
flip1995
pushed a commit
to flip1995/rust
that referenced
this issue
Sep 12, 2023
[`implied_bounds_in_impls`]: include (previously omitted) associated types in suggestion Fixes rust-lang#11435 It now includes associated types from the implied bound that were omitted in the second bound. Example: ```rs fn f() -> impl Iterator<Item = u8> + ExactSizeIterator> {..} ``` Suggestion before this change: ```diff - pub fn my_iter() -> impl Iterator<Item = u32> + ExactSizeIterator { + pub fn my_iter() -> impl ExactSizeIterator { ``` It didn't include `<Item = u32>` on `ExactSizeIterator`. Now, with this change, it does. ```diff - pub fn my_iter() -> impl Iterator<Item = u32> + ExactSizeIterator { + pub fn my_iter() -> impl ExactSizeIterator<Item = u32> { ``` We also now extend the span to include not just possible `+` ahead of it, but also behind it (an example for this is in the linked issue as well). **Note:** The overall diff is a bit noisy, because building up the suggestion involves quite a bit more logic now and I decided to extract that into its own function. For that reason, I split this PR up into two commits. The first commit contains the actual "logic" changes. Second commit just moves code around. changelog: [`implied_bounds_in_impls`]: include (previously omitted) associated types in suggestion changelog: [`implied_bounds_in_impls`]: include the `+` behind bound if it's the last bound
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The text was updated successfully, but these errors were encountered: