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

Update Clippy #94329

Merged
merged 85 commits into from
Feb 26, 2022
Merged

Update Clippy #94329

merged 85 commits into from
Feb 26, 2022

Conversation

flip1995
Copy link
Member

flip1995 and others added 30 commits February 10, 2022 18:40
…sakis"

This reverts commit e7cc3bd, reversing
changes made to 734368a.
Migrate `dbg_macro` to late pass

changelog: Make `dbg_macro` work with crate level attributes and inside macro calls

One down for rust-lang#6610, fixes rust-lang#7275

Also fixes rust-lang#7274, and adds parenthesis around the suggestion for `dbg!(1, 2, 3)` as it expands to a tuple
Revert lazy TAIT PR

Revert rust-lang#92306 (sorry `@Aaron1011,` will include your changes in the fix PR)
Revert rust-lang#93783
Revert rust-lang#92007

fixes rust-lang#93788
fixes rust-lang#93794
fixes rust-lang#93821
fixes rust-lang#93831
fixes rust-lang#93841
…, r=llogiq

Downgrade transmute_undefined_repr to nursery

Reason: rust-lang#8417. I am skeptical of this lint but maybe there is a narrower subset of types on which it is useful, so keeping it for now but moving to nursery for further development.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Remove [`transmute_undefined_repr`] from default set of enabled lints
Replace a few paths with diagnostic items

A fairly small change towards rust-lang#5393

changelog: none
Document `pub` requirement for `new_without_default` lint

fixes rust-lang#8415

Also adds some UI tests that ensure that `pub` is required on both the struct _and_ the field. The only thing I'm not sure about is that the lint actually [checks](https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/new_without_default.rs#L102) if `new` is _reachable_, not _public_. To the best of my understanding, both the struct and the method need to be public for the method to be reachable for external crates (I certainly didn't manage to craft a counterexample).

changelog: Document `pub` requirement for ``[`new_without_default`]`` lint.
Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs

Fixes: rust-lang#8417

The description has also been made more precise.

changelog: Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs
changelog: Move `transmute_undefined_repr` back to `correctness`
Make `Res::SelfTy` a struct variant and update docs

I found pattern matching on a `(Option<DefId>, Option<(DefId, bool)>)` to not be super readable, additionally the doc comments on the types in a tuple variant aren't visible anywhere at use sites as far as I can tell (using rust analyzer + vscode)

The docs incorrectly assumed that the `DefId` in `Option<(DefId, bool)>` would only ever be for an impl item and I also found the code examples to be somewhat unclear about which `DefId` was being talked about.

r? `@lcnr` since you reviewed the last PR changing these docs
The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug
inside any format string in the same impl
The to_string_in_display check is kept as is - like in the
format_in_format_args lint

For now only Display and Debug are checked
This could also be extended to other Format traits (Binary, etc.)
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
  means we can move a lot of methods away from `TyS`, leaving `TyS` as a
  barely-used type, which is appropriate given that it's not meant to
  be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
  E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
  than via `TyS`, which wasn't obvious at all.

Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs

Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
  `Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
  which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
  of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
  (pointer-based, for the `Equal` case) and partly on `TyS`
  (contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
  or `&`. They seem to be unavoidable.
Specifically, change `Region` from this:
```
pub type Region<'tcx> = &'tcx RegionKind;
```
to this:
```
pub struct Region<'tcx>(&'tcx Interned<RegionKind>);
```

This now matches `Ty` and `Predicate` more closely.

Things to note
- Regions have always been interned, but we haven't been using pointer-based
  `Eq` and `Hash`. This is now happening.
- I chose to impl `Deref` for `Region` because it makes pattern matching a lot
  nicer, and `Region` can be viewed as just a smart wrapper for `RegionKind`.
- Various methods are moved from `RegionKind` to `Region`.
- There is a lot of tedious sigil changes.
- A couple of types like `HighlightBuilder`, `RegionHighlightMode` now have a
  `'tcx` lifetime because they hold a `Ty<'tcx>`, so they can call `mk_region`.
- A couple of test outputs change slightly, I'm not sure why, but the new
  outputs are a little better.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.

Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
  we need separate arena for it, because we can't use the `Dropless` one any
  more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
There's still open discussion if this lint is ready to be enabled by
default. We want to give us more time to figure this out and prevent
this lint from getting to stable as an enabled-by-default lint.
…oup, r=dtolnay

Move transmute_undefined_repr back to nursery

There's still open discussion if this lint is ready to be enabled by
default. We want to give us more time to figure this out and prevent
this lint from getting to stable as an enabled-by-default lint.

cc rust-lang/rust-clippy#8432

r? `@Manishearth` `@dtolnay`

I think this is the way to go here. We can re-enable this lint with the next sync, if we should decide to do so. But I would hold of for this release.

We have until Friday (beta branching) to decide if we want to merge this.
It should only include the identifier, or misspelling suggestions will be wrong.
…steffen

new lint: `recursive_format_impl`

The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug inside any format string in the same impl
The to_string_in_display check is kept as is - like in the format_in_format_args lint

This is my first contribution so please check it for better / more idiomatic checks + error messages. Note the format macro paths are shared with the `format_in_format_args` lint - maybe those could be moved to clippy utils too.

This relates to issues rust-lang#2691 and rust-lang#7830

------

changelog: Renamed `to_string_in_display` lint to [`recursive_format_impl`] with new check for any use of self as Display or Debug inside the same format trait impl.
* Lint when slicing triggers auto-deref
* Lint when slicing returns the same type as dereferencing
Replace some more paths with diagnostic items

cc rust-lang#5393

Replaces the macro & mem paths, and catches a couple others that were unused

changelog: none
@flip1995
Copy link
Member Author

Yes x.py check uses the bootstrap compiler. That's why I didn't mention this let_chains issue last time. I figured one the bootstrap compiler is bumped, the issue would go away.

But since #94290 isn't merged yet, it is probably still the same issue. I will check again once that is merged.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 25, 2022
…=Dylan-DPC

Fix debug_assert in unused lint pass

This fixes a debug assertion in the unused lint pass. As a side effect, this also improves the span generated for tuples in the `unused_must_use` lint.

found in rust-lang#94329

A reproducer for this would be

```rust
fn main() { (1, (3,)); }
```

Not sure, if I should add a regression test for a `debug_assert`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 25, 2022
…=Dylan-DPC

Fix debug_assert in unused lint pass

This fixes a debug assertion in the unused lint pass. As a side effect, this also improves the span generated for tuples in the `unused_must_use` lint.

found in rust-lang#94329

A reproducer for this would be

```rust
fn main() { (1, (3,)); }
```

Not sure, if I should add a regression test for a `debug_assert`.
Alexendoo and others added 4 commits February 25, 2022 21:10
Add `print_in_format_impl` lint

changelog: new lint: [`print_in_format_impl`]

Lints the use of `print`-like macros in manual `Display`/`Debug` impls. I feel like I make this mistake every time I write one 😄

r? `@camsteffen`
…amsteffen

fix false positives of large_enum_variant

fixes: rust-lang#8321
The size of enums containing generic type was calculated to be 0.
I changed [large_enum_variant] so that such enums are not linted.

changelog: none
Fix `ptr_arg`

fixes: rust-lang#8463

changelog: Fix `ptr_arg` when multiple arguments are being checked in one function
@matthiaskrgr
Copy link
Member

bootstrap bump has merged :)

@flip1995 flip1995 closed this Feb 26, 2022
@flip1995 flip1995 reopened this Feb 26, 2022
@flip1995
Copy link
Member Author

Alright. re-synced. let_chains seem to work now 👍

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 26, 2022

📌 Commit 0d4e583 has been approved by Manishearth

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 26, 2022
@bors
Copy link
Contributor

bors commented Feb 26, 2022

⌛ Testing commit 0d4e583 with merge 8604ef0...

@bors
Copy link
Contributor

bors commented Feb 26, 2022

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing 8604ef0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 26, 2022
@bors bors merged commit 8604ef0 into rust-lang:master Feb 26, 2022
@rustbot rustbot added this to the 1.61.0 milestone Feb 26, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8604ef0): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@flip1995 flip1995 deleted the clippyup branch February 27, 2022 19:27
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 3, 2022
Move transmute_undefined_repr back to nursery again

This PR reapplies rust-lang#94014, which was reverted unintentionally I think by rust-lang#94329. The combination of rust-lang/rust-clippy#8432 + rust-lang/rust-clippy#8497 in clippy should prevent this from happening again.

r? `@Manishearth`
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.