Skip to content

Conversation

ada4a
Copy link
Contributor

@ada4a ada4a commented Aug 21, 2025

continuation of #15519, see there for the commit structure

AFAICT this finally resolves #7784

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 21, 2025
@ada4a
Copy link
Contributor Author

ada4a commented Aug 22, 2025

While working on this, I've noticed two common patterns:

  1. if-else-chains with repeated is_diagnostic_item checks:
    if cx.tcx.is_diagnostic_item(sym::ptr_null, method_defid) {
        OmitFollowedCastReason::Null(qpath)
    } else if cx.tcx.is_diagnostic_item(sym::ptr_null_mut, method_defid) {
        OmitFollowedCastReason::NullMut(qpath)
    } else {
        OmitFollowedCastReason::None
    which can be replaced with a match:
     match cx.tcx.get_diagnostic_name(method_defid) {
         Some(sym::ptr_null) => OmitFollowedCastReason::Null(qpath),
         Some(sym::ptr_null_mut) => OmitFollowedCastReason::NullMut(qpath),
         _ => OmitFollowedCastReason::None,
     }
  2. ||-d is_diagnostic_item checks:
    cx.tcx.is_diagnostic_item(sym::cmp_ord_min, id) || cx.tcx.is_diagnostic_item(sym::cmp_ord_max, id)
    which I guess can be replaced with this (recommended in Use get_diagnostic_name #7784):
    matches!(cx.tcx.get_diagnostic_name(id), Some(sym::cmp_ord_min | sym::cmp_ord_max))
    or, given that these usually appear in let-chains, with something like this:
    if let Some(name) = cx.tcx.get_diagnostic_name(id)
    && matches!(name, sym::cmp_ord_min | sym::cmp_ord_max)
    which I personally like better and is what I used throughout the PRs, though you can of course disagree

And I wonder whether I should try making this into internal lints, even if without the suggestions at first?

@ada4a ada4a force-pushed the get-diagnostic-item-2 branch from c6282ae to 3c3dc6d Compare August 22, 2025 13:31
@samueltardieu
Copy link
Member

And I wonder whether I should try making this into internal lints, even if without the suggestions at first?

Why not? I also systematically suggest using the more efficient form when I do reviews, but I didn't go through the existing code. If this is not too much work, you might attempt it.

@samueltardieu samueltardieu added this pull request to the merge queue Aug 23, 2025
Merged via the queue into rust-lang:master with commit f35c203 Aug 23, 2025
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 23, 2025
@ada4a ada4a deleted the get-diagnostic-item-2 branch August 24, 2025 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use get_diagnostic_name

4 participants