Prefer pub(super) in unreachable_pub lint suggestion

[Urgau]

This PR updates the unreachable_pub lint suggestion to prefer pub(super) instead of pub(crate) when possible.

cc @petrochenkov
r? @nnethercote

[Urgau]

Seems like, one can't reexport a pub(super) items, this a annoying.

mod go {
    pub(super) use fpu_precision::set_precision;

    mod fpu_precision {
        pub(super) fn set_precision<T>() {} // should be `pub(crate)`
    }
}

[Urgau]

I've completely reworked the logic to better use the effective visibility.

I've also locally tested the new logic on the core, alloc, panic_unwind and std crates with ./x.py fix and the new suggestions all successfully passed.

@rustbot ready

[nnethercote]

FWIW, I personally find the difference between pub(crate) and pub(super) to be of very little importance in practice, and I almost always use pub(crate). (In contrast, I find the difference between pub(crate|super) and pub is much more important, as is the difference between pub(crate|super) and no visibility.)

Is this change worth the extra complexity?

[petrochenkov]

It's written in a very wordy way, but it's really just one condition when we reset the default pub(crate) recommendation to pub(super), I think it's worth it (after refactoring).

[nnethercote]

r? @petrochenkov

[Urgau]

I've reduced it all to a single (somewhat small) if, not sure if it's better, but it's much less verbose as asked.

[nnethercote]

Have you done a stage 2 build so that the new behaviour is applied to the compiler's own code? Presumably it would require making a lot of additional changes, because warn(unreachable_pub) is used in most of the compiler's own crates.

[Urgau]

I haven't tested it on the compiler but as said in #132426 (comment) I have tested it for core, alloc, panic_unwind and std; and since then I have also successfully tested it on hyper, html5ever and itertools, no issues automatically applying the suggestions.

[nnethercote]

How many changes were necessary?

[Urgau]

I don't really understand the question, but what I did is just a cargo fix (or equivalent) for each of them with RUSTFLAGS="-Wunreachable_pub".

I didn't have to fix anything after the cargo fix¹. The pub(super) were all as far as I can tell functional.

Footnotes

1. I did have some issues with rustfix and cargo fix it-self, but those are not related to the lint or it's suggestions. ↩

[nnethercote]

The question is how big is the diff obtained when running cargo fix? I.e. how many pub(crate) occurrences were changed to pub(super)? My guess is there would be a lot.

[nnethercote]

After discussing on Zulip: I was mistakenly thinking that this would suggest pub(super) for pub(crate) occurrences, but it only suggests pub(super) for pub occurrences. So I have no objections to that.

[petrochenkov]

@bors r+

[bors]

📌 Commit 0bc622d has been approved by petrochenkov

It is now in the queue for this repository.