Replace some matches! with if let or ==

[estebank]

No description provided.

[rustbot]

Some changes occurred to constck

cc @fee1-dead

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in exhaustiveness checking

cc @Nadrieril

rustc_errors::emitter was changed

cc @Muscraft

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in check-cfg diagnostics

cc @Urgau

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

Some changes occurred in need_type_info.rs

cc @lcnr

Some changes occurred in compiler/rustc_ast/src/expand/autodiff_attrs.rs

cc @ZuseZ4

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rust-log-analyzer]

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

Click to see the possible cause of the failure (guessed by this bot)

[RUSTC-TIMING] unicode_segmentation test:false 0.259
[RUSTC-TIMING] askama_derive test:false 1.271
    Checking askama v0.14.0
[RUSTC-TIMING] askama test:false 0.165
error[E0369]: binary operation `==` cannot be applied to type `cfg::Format`
   --> src/librustdoc/clean/cfg.rs:228:33
    |
228 |         let mut msg = if format == Format::LongHtml {
    |                          ------ ^^ ---------------- cfg::Format
    |                          |
    |                          cfg::Format
    |
note: an implementation of `std::cmp::PartialEq` might be missing for `cfg::Format`
   --> src/librustdoc/clean/cfg.rs:403:1

[RalfJung]

What is the reason for this change? What I like about matches! is that I can use it without worrying about whether the type implements PartialEq. Also, with if let I have to carefully check the condition to see if anything is being bound; matches! makes it clear that nothing is bound.

So while I don't really mind any of these diffs, it also doesn't look like a clear improvement that's worth a sweeping change, and new "unnecessary" matches! will inevitably be added again.

[jdonszelmann]

agreed, not sure this is necessarily better.

[nnethercote]

I don't mind the change. if let/== is shorter than matches!, and using built-in features rather than macros is nice.

I do think a PR touching 82 files deserves a description that explains the how and the why, though. What prompted this? Did you automate any of it? When did you use == vs if let?

[estebank]

What is the reason for this change?

I do think a PR touching 82 files deserves a description that explains the how and the why, though. What prompted this? Did you automate any of it? When did you use == vs if let?

That's fair! I was touching something else and noticed a matches! that was present most likely only because let chains didn't exist at the time, so I did a quick if matches! search and went through them manually while I was at an airport and published the PR with the intention to start a conversation about whether we should prioritize this new style or not (and then proceeded not to start it). As alluded by Ralf, I tried to use == whenever PartialEq was already implemented and avoided adding it to those that didn't (even though they probably should, like those that are purely Yes/No enums).

What I like about matches! is that I can use it without worrying about whether the type implements PartialEq. Also, with if let I have to carefully check the condition to see if anything is being bound; matches! makes it clear that nothing is bound.

So while I don't really mind any of these diffs, it also doesn't look like a clear improvement that's worth a sweeping change, and new "unnecessary" matches! will inevitably be added again.

I went through these manually because I wanted to get a sense for whether making a clippy or rustc lint for this would be useful. I see that clippy has almost the opposite, when a match expression could be replaced with matches!, and for when if let could have been == instead, but none for the case touched in this PR.

---

Regardless, the reason I created the PR was to gather feedback: is the style shown here what we'd want? I do think that if we have to keep matches! around it shows a lang expressiveness limitation. I am not too convinced by the argument about having to double check whether there are new bounds introduced (but understand where it comes from). Seeing matches!(expr, Pat if cond) was what made me do this, because that should really be an if let chain most of the time, IMO. The only cases where I saw that matches! is still clearly necessary are 1) when wanting to be stable-compatible and would need an if let chain in a match arm condition and 2) when you have || instead of a let chain.