Internal lint: Ban pub re-exports

[Centril]

Suggested lint name: rustc::pub_reexport.

When given something like the following, reject it (as a lint):

pub use syntax_pos::symbol;

cc #49509

---

Me and @Mark-Simulacrum were discussing library-ification and I mentioned to them how re-exports like this (specifically of syntax_pos::hygiene in the case of a refactoring I'm working on) was detrimental because it encourages relying on wider dependencies than strictly necessary. We want to fix the situation in #65031 and having this lint could help with that.

@Mark-Simulacrum had the idea of introducing an internal lint to ban it. This will cause some large diffs for syntax_pos::symbol:: in particular but that should be gradually fixable on a crate-by-crate basis through #![allow(...)] and it is mostly mechanical.

cc @oli-obk @nikomatsakis

[eddyb]

That specific example made me wonder if syntax_pos should be merged with rustc_lexer or be its own rustc_source_map or something along those lines. syntax_pos doesn't feel like it fits anywhere.

And Symbol specifically belongs in rustc_lexer, even if Span may not.

cc @petrochenkov @matklad

[petrochenkov]

I actually wanted to do the opposite thing and reexport the whole syntax_pos from syntax for convenience.
There are almost no crates that depend on the former but don't need to depend the latter (rustc_errors?).

And Symbol specifically belongs in rustc_lexer, even if Span may not.

That includes the symbol interner.
(Unless rustc_lexer provides only the Symbol type but not a way to create it.)
Do other users of rustc_lexer need this global state?

[petrochenkov]

I agree that it would be nice to cleanup random public reexports that are traces of various refactorings rather than conscious choices.
E.g. I recently removed some of those in rustc_metadata in d17296d.

[matklad]

And Symbol specifically belongs in rustc_lexer, even if Span may not.

Hm, I don't see this :) I see this as a very happy accident that rustc_lexer can exist as a separate crate as a pure algorithm without exposing any data structures at all.

Sharing data types would be a step back. Sharing state (the interner) would be even more problematic.

[eddyb]

Oh, my bad, I forgot that rustc_lexer can function without Span or Symbol.

[nikomatsakis]

I would be ok with this change. Even leaving aside the library-ification question, I personally find the "facade" style sort of confusing -- I'm never quite sure which name I should be importing. I'd prefer if we had one canonical path that we used everywhere. I also like to use rg as a simple "find all uses", and having more than one path by which something can be named makes that harder.

[RalfJung]

In Miri we currently make heavy use of public re-exports. Namely, librustc_mir::interpret re-exports all of librustc::mir::interpret. I would prefer if we could keep it that way: The split of Miri between these two crates is mostly because some Miri types are needed in the wider compiler and hence have to live in librustc; when working inside the interpreter I generally could not care less about whether some type is defined here or there.

OTOH, for hir/syntax-level things, I have often been confused myself by all the reexports. When seeing two types of the same name (e.g. in the docs, comparing to what I have in my code), I never know if those are genuinely different or if one is a reexport of the other. So maybe the lesson is that the Miri part works for me just because I know better? I am not sure.

[Centril]

In Miri we currently make heavy use of public re-exports. Namely, librustc_mir::interpret re-exports all of librustc::mir::interpret. I would prefer if we could keep it that way: [...]

Would you be OK with just #[allow(...)]ing the lint for this specific re-export for now?

[RalfJung]

Sure, the question is more if you'd be okay with that. ;)

[Centril]

Yeah sure, doesn't have to be all / at once or nothing with internal lints. :)

[jyn514]

I tried adding this (rust-lang/compiler-team#368) and it was rejected because it was too intrusive and people were used to using the re-exports. Does it still make sense to keep this open?