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 #94929

Merged
merged 97 commits into from
Mar 14, 2022
Merged

Update Clippy #94929

merged 97 commits into from
Mar 14, 2022

Conversation

flip1995
Copy link
Member

r? @Manishearth

A few days delayed, because I recovered from a cold last week and couldn't get myself to do the sync, sorry. 🙃

CrazyRoka and others added 30 commits December 16, 2021 18:57
by using a static data structure to track transitive relations.
- fix code that have variables that is "only used in recursion"
- add test
…rsion

# Conflicts:
#	clippy_lints/src/lib.rs
…-in macros

Previously it always returned `MacroKind::Bang` while some of those macros are actually attributes and derives
…error-span, r=jackh726

better ObligationCause for normalization errors in `can_type_implement_copy`

Some logic is needed so we can point to the field when given totally nonsense types like `struct Foo(<u32 as Iterator>::Item);`

Fixes rust-lang#93687
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".

That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have:
* `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics
  * can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation
  * asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`)
  * `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called
    (though note that this isn't a real guarantee until after completing the work on
     rust-lang#69426)
* `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.)
  * can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()`

This PR is a companion to other ongoing work, namely:
* rust-lang#69426
  and it's ongoing implementation:
  rust-lang#93222
  the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes
* rust-lang#93244
  would make the choices of API changes (esp. naming) in this PR fit better overall

In order to be able to let `.emit()` return anything trustable, several changes had to be made:
* `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail)
  * it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()`
* `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic
  * it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user
  * this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it
  * warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early)
  * `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`)
* since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>,
  we have to make `.emit()` idempotent wrt the guarantees it returns
  * thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission
* `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic)
  * the APIs were already mostly identical, allowing for low-effort porting to this new setup
  * only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in rust-lang#93259)
  * `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective)
  * `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same)

This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each.

r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
J-ZhengLi and others added 15 commits March 10, 2022 09:44
and change its lint group to "complexity"
…les, r=camsteffen

Improve styles of filtering options for Clippy's lint list

Partially solves rust-lang#7958

Updated styles for filtering options. It now uses dropdown menus.

![image](https://user-images.githubusercontent.com/19844144/144608479-cdd9de0b-f101-4d49-a135-0969efb01a11.png)

changelog: none
This commit makes `AdtDef` use `Interned`. Much the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
new lint that detects useless match expression

fixes rust-lang#7040

changelog: Add new  lint [`needless_match`] under complexity lint group
…logiq

new lint: `only_used_in_recursion`

changed:
- added `only_used_in_recursion`.
- fixed code that variables are only used in recursion.
- this would not lint when `unused_variable`

This fixes: rust-lang#8390

-----

changelog: add lint [`only_used_in_recursion`]
Rustup

r? `@ghost`

changelog: none
@rust-highfive
Copy link
Collaborator

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 14, 2022
@Manishearth
Copy link
Member

@bors r+ p=2 rollup=never

@bors
Copy link
Contributor

bors commented Mar 14, 2022

📌 Commit 706fa54 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2022
@bors
Copy link
Contributor

bors commented Mar 14, 2022

⌛ Testing commit 706fa54 with merge 285fa7e...

@bors
Copy link
Contributor

bors commented Mar 14, 2022

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing 285fa7e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 14, 2022
@bors bors merged commit 285fa7e into rust-lang:master Mar 14, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 14, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (285fa7e): 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

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.

None yet