Don't show two different types as the same thing in a single function

[scottmcm]

I noticed this in https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_ssa/traits/consts/trait.ConstMethods.html#tymethod.scalar_to_backend today:

Those two Scalars are actually different types, however.

It would be nice to detect things like this and show some distinguisher. Maybe value::Scalar and rustc_abi::Scalar -- including enough module context to distinguish -- or something? Or maybe show the crate name?

(Crate name might be good, since a single crate can fix multiple types with the same name, if needed.)

[krtab]

@rustbot claim

(cc @GuillaumeGomez )

[fmease]

Prior art: trimmed_def_paths.

[krtab]

I don't think trimmed_def_paths does exactly what we want, because in our case, we should trim based on what has already been displayed.

MRE

pub mod Foo {
    pub enum E {}
}

pub mod Bar {
    pub enum E {}
}


pub fn f(foo: Foo::E, bar: Bar::E) {}

gives in doc

pub fn f(foo: E, bar: E)

Now the question is, should it be:

pub fn f(foo: E, bar: Foo::E)

pub fn f(foo: Bar::E, bar: E)

or

pub fn f(foo: Bar::E, bar: Foo::E)

[fmease]

I think the latter (or was that a rhetorical question lol?)

[GuillaumeGomez]

I also prefer the latter (whether it's a rhetorical question or not).

[krtab]

I prefer it as well, but as discussed in #122872 this means that there must be two passes to disambiguate: the first one identifies the non ambiguous suffix of the symbol path, and the second one does the actual rendering. Choosing pub fn f(foo: E, bar: Foo::E) allows us to do it in one pass (a sort of "streaming" disambiguation), which makes it easier to generalize: printing functions must now simply take a context and reset it when appropriate. This context can easily be passed along when we match on the constructs.

If we do the two passes solution, the matching/deconstructing code has to be duplicated (or we go for a full visitor pattern?)

[GuillaumeGomez]

What information is missing to force it to be done in two passes? Once you have all names in the same context (ie, function arguments, struct fields, etc), you can check that a same type has the same name or not and there disambiguate. If you don't have a context when displaying, then try generating the name ahead of time if needed in the clean pass?

[wackbyte]

Here's some notes I have on how I think disambiguation should work:

• The least possible context should be used to disambiguate.
  • i.e., a::b::c::T and a::d::c::T disambiguate as b::c::T and d::c::T.
  • What about types from the crate root? Perhaps $crate::T?
    • What if there were extern crate foo and mod foo? ::foo::T and foo::T? foo::T and $crate::foo::T?
  • self, super, and paths to re-exports should be preserved for disambiguation.
    • i.e., paths should not necessarily be canonicalized.
• The name of an item should be prioritized on its own page.
  • i.e., struct Foo(Foo, other::Foo) should preserve the first Foo in Foo's page.
    • This would not occur in the "Aliased type" section of the page of a type alias using Foo, though, because it would not be Foo's page.
  • The same should occur in the implementations on a trait's page.
• Type parameters should be accounted for.
  • i.e., <T = a::T> should not render as <T = T>.
• Disambiguation on item pages should work in one of the following ways:
  1. Local to the item definition, each implementation, and each associated item.
     • i.e., a::T and b::T in different methods in the same implementation can coexist as T.
  2. Local to the item definition and each implementation.
     • i.e., a::T and b::T in different implementations on the same page can coexist as T.
     • I prefer this one.
  3. Global to the entire page.
     • i.e., any types with the same name referenced on a page will always be disambiguated.
     • This one is the most consistent, but perhaps unnecessarily so?