Stream: rustdoc

Topic: What is the difference between AssocTypeItem and TypedefItem


view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:05):

@GuillaumeGomez do you know the difference between AssocTypeItem and TypedefItem(_, true)?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:05):

I would like to get rid of one or the other (preferably TypedefItem(_, true))

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 17:06):

TypedefItem is an AssocTypeItem for me. I discovered that it was badly used at the same time as you today haha

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:06):

hmm, ok

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:06):

I'll delete it and see what goes wrong :laughing:

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 17:07):

I approve this way of doing things XD

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:37):

I don't understand this well enough to fix it. There's different info in each: TypedefItem holds

while AssocTypeItem only has the generic bounds and a place for the type default.

There's all sorts of things wrong here:

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:38):

maybe I'll start small and just remove the cleaned version from TypedefItem

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:41):

wow this logic is just so convoluted

view this post on Zulip Joshua Nelson (Jan 03 2021 at 17:43):

impl Clean for hir::Impl sees that it's an item, calculates the def id, passes it to build_ty, recalculates the type, passes it to build_type_alias_item, which then passes it back to build_ty, calculates the type I think a fourth time now, and only then actually does work in impl Clean for Ty

view this post on Zulip Joshua Nelson (Jan 03 2021 at 18:05):

@GuillaumeGomez here's what goes wrong if I remove all that and just clone type_:

comparing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/test/rustdoc/deref-typedef.nightly/foo/struct.Bar.html ⟶   /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/test/rustdoc/deref-typedef/foo/struct.Bar.html
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

41
        <div class="block items">
          <a class="sidebar-title" href="#deref-methods">Methods from Deref&lt;Target=FooC&gt;</a>
          <div class="sidebar-links">
-           <a href="#method.foo_a">foo_a</a><a href="#method.foo_b">foo_b</a><a href="#method.foo_c">foo_c</a>
+           <a href="#method.foo_c">foo_c</a>
          </div><a class="sidebar-title" href="#trait-implementations">Trait Implementations</a>
          <div class="sidebar-links">
            <a href="#impl-Deref">Deref</a>

91
        Methods from <a class="trait" href="https://doc.rust-lang.org/nightly/core/ops/deref/trait.Deref.html" title="trait core::ops::deref::Deref">Deref</a>&lt;Target = <a class="type" href="../foo/type.FooC.html" title="type foo::FooC">FooC</a>&gt;<a href="#deref-methods" class="anchor"></a>
      </h2>
      <div class="impl-items">
-       <h4 id="method.foo_a" class="method">
-         <code>pub fn <a href="#method.foo_a" class="fnname">foo_a</a>(&amp;self)</code><a class="srclink" href="../src/foo/deref-typedef.rs.html#18" title="goto source code">[src]</a>
-       </h4>
-     </div>
-     <div class="impl-items">
-       <h4 id="method.foo_b" class="method">
-         <code>pub fn <a href="#method.foo_b" class="fnname">foo_b</a>(&amp;self)</code><a class="srclink" -ref="../src/foo/deref-typedef.rs.html#22" title="goto source code">[src]</a>
-       </h4>
-     </div>
-     <div class="impl-items">
        <h4 id="method.foo_c" class="method">

view this post on Zulip Joshua Nelson (Jan 03 2021 at 18:06):

and I'm very unclear why that happens

view this post on Zulip Joshua Nelson (Jan 03 2021 at 18:06):

git says you were the person who added it in https://github.com/rust-lang/rust/commit/12f029b7eecc01a38fbeec0eebe9041aa1bab7a5

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:09):

well, to be fair, I added around 70% of rustdoc source code soooo... Let me check if I can remember this part.

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:11):

Ok I remember: it's because we need the derefed type

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:11):

if we don't get it, we can't get its methods

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:12):

This is why you have the missing methods when you removing it.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 18:13):

but isn't type_ already the deref type?

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:13):

Apparently not, I'm still checking that

view this post on Zulip Joshua Nelson (Jan 03 2021 at 18:13):

I'm confused what calling type_of actually does

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:14):

btw, the full PR is here: https://github.com/rust-lang/rust/pull/68093 and it's actually quite recent

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:15):

So, I think we use Typedef because it's kinda the same logic between deref item and type alias

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 18:15):

not very beautiful though...

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:18):

so I found there are two differences between type_ and item_type:

  1. Res is FooA for type_ and Err for item_type
  2. the path segments are FooB for type_ and FooA for item_type

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:18):

I think the fix is that render shouldn't care about the path segments

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:19):

oh wait no I'm dumb, the third and largest difference is the DefIds: FooB for type_ and FooA for item_type

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:27):

I think this might actually work by accident

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:27):

one sec

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:30):

oh no I'm just very confused

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:39):

Let's just say I'm not very proud of that :-°

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:40):

ok yes I was right, I confused myself.
@GuillaumeGomez this doesn't show any methods at all on Bar:

pub struct FooA;
pub type FooB = FooA;
pub type FooC = FooB;
pub type FooD = FooC;
pub type FooE = FooD;
pub type FooF = FooE;
pub type FooG = FooF;
pub type FooH = FooG;
pub type FooI = FooH;
pub type FooJ = FooI;

impl FooA {
    pub fn foo_a(&self) {}
}

impl FooB {
    pub fn foo_b(&self) {}
}

impl FooC {
    pub fn foo_c(&self) {}
}

pub struct Bar;
impl std::ops::Deref for Bar {
    type Target = FooJ;
    fn deref(&self) -> &Self::Target { unimplemented!() }
}

What's going on is that each time you call ty.clean().def_id(), you get a different ID than before: it returns the ID of the item it's aliased to, not the original. So because you call ty.clean().def_id().type_of().clean().def_id() it works for two nested levels of aliases. But it breaks on anything more than two.

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:40):

Yup

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:41):

ok well good, that means that this can be done much more simply

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:42):

recursiooooooon

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:42):

well even simpler than that I think

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:43):

rustdoc::clean::impl_Clean_for_TyAlias
├─0ms DEBUG rustdoc::clean def_id=DefId(0:5 ~ foo[8787]::FooB), ty=Ty {
│     hir_id: HirId {
│         owner: DefId(0:5 ~ foo[8787]::FooB),
│         local_id: 1,
│     },
│     kind: Path(
│         Resolved(
│             None,
│             Path {
│                 span: src/test/rustdoc/deref-typedef.rs:14:17: 14:21 (#0),
│                 res: Def(
│                     Struct,
│                     DefId(0:3 ~ foo[8787]::FooA),
│                 ),

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:43):

oh wait this is for FooB, not FooC

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:43):

so yeah I guess recursion :(

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:44):

I was really wondering for one sec how you could get it without recursing

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:44):

well I would expect there to be a compiler API for this

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:44):

since it happens often, is expensive, and can easily be cached by short-circuiting

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:45):

I guess there is one. But we still need to get all implementations made on each alias if any

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:45):

I would expect those to be present on the final DefId

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:45):

it doesn't make sense for the API to work any other way

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 19:47):

Well, sounds like a great new way to get things done :D

view this post on Zulip Joshua Nelson (Jan 03 2021 at 19:59):

oh I bet it's normalize()

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:00):

@lcnr If I have type B = A; type C = B; ..., will normalize() turn Z into A?

view this post on Zulip lcnr (Jan 03 2021 at 20:09):

you won't even have to normalize here, do you?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:09):

what's the easier way?

view this post on Zulip lcnr (Jan 03 2021 at 20:09):

the moment you convert to ty::Ty you should already end up with A

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:09):

... I should?

view this post on Zulip lcnr (Jan 03 2021 at 20:09):

type aliases don't really exist after hir

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:09):

that's not the behavior I'm seeing but maybe I'm just confused

view this post on Zulip lcnr (Jan 03 2021 at 20:10):

hmm, maybe you are still using hir::Ty?

view this post on Zulip lcnr (Jan 03 2021 at 20:10):

or i am really confused rn :sweat_smile:

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:11):

well all this code is a mess really lol https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/What.20is.20the.20difference.20between.20AssocTypeItem.20and.20TypedefItem/near/221462182

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:11):

let me start by just calling type_of and see if it helps

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:14):

ok you are correct that it is A before I ever pass it to normalize

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:14):

which makes me think this is a bug elsewhere in rustdoc

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:29):

anddd I found it, rustdoc does this in three places as usual :face_palm:

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index f4eb1924e6f..d78ae11fc78 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -1118,10 +1118,11 @@ fn clean(&self, cx: &DocContext<'_>) -> Item {
                     }
                     MethodItem(m, Some(self.defaultness))
                 }
-                hir::ImplItemKind::TyAlias(ref ty) => {
-                    let type_ = ty.clean(cx);
-                    let item_type = type_.def_id().and_then(|did| inline::build_ty(cx, did));
-                    TypedefItem(Typedef { type_, generics: Generics::default(), item_type }, true)
+                hir::ImplItemKind::TyAlias(ref hir_ty) => {
+                    let type_ = hir_ty.clean(cx);
+                    let item_type = hir_ty_to_ty(cx.tcx, hir_ty).clean(cx);
+                    TypedefItem(Typedef { type_: type_.clone(), generics: Generics::default(), item_type: Some(item_type) }, true)
                 }
             };
             Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx)

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:31):

thanks for the help everyone :)

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:00):

I have an idea to simplify this further, but unfortunately it means the bug in https://github.com/rust-lang/rust/issues/80379 will apply to the current crate as well :(

view this post on Zulip lcnr (Jan 03 2021 at 21:05):

if you don't mind looking into type aliases in rustdoc, can you convert all tys to ty::Ty and then walk the substs for Adts backwards during printing to remove all arguments which are equal to the default param?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:07):

lcnr said:

if you don't mind looking into type aliases in rustdoc, can you convert all tys to ty::Ty and then walk the substs for Adts backwards during printing to remove all arguments which are equal to the default param?

that will break if someone explicitly writes Vec<usize, Global>, though, right?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:07):

I want to replace it only if it's been injected by the compiler

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:08):

I guess there's not really a way to do that across crates

view this post on Zulip lcnr (Jan 03 2021 at 21:08):

yeah

view this post on Zulip lcnr (Jan 03 2021 at 21:08):

though is printing Vec<usize, Global> desirable even if someone explicitly prints it?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:08):

yeah, it probably makes sense to elide it no matter what

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:08):

I don't have time to do this today probably but that sounds like the right approach :)

view this post on Zulip lcnr (Jan 03 2021 at 21:09):

changing the param defaults is a breaking change so there shouldn't be too many reasons to explicitly specify the param default

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:11):

I can imagine it being confusing if hashbrown changes the default hasher across releases or something

view this post on Zulip Joshua Nelson (Jan 03 2021 at 21:11):

but that still seems better than just never eliding it

view this post on Zulip lcnr (Jan 03 2021 at 21:13):

yeah, it is a bit unfortunate


Last updated: Oct 11 2021 at 22:34 UTC