rust-lang/rust

Intra-rustdoc links do not work for all primitives

nixpulvis opened this issue · 11 comments

nixpulvis commented

Coming from the tracking issue #43466.

I'm trying to link to slice::rotate_left and it doesn't seem to be supported I tested pointer::is_null as well, to test if it's just polymorphic primitives, and it seems that might be the issue, since it also doesn't work as a intra-link.

/cc @GuillaumeGomez

GuillaumeGomez commented

I'll recheck it and see what's going on.

  • 👍1
GuillaumeGomez commented

The problem here is that, even if being primitive types, they're not considered as is in the compiler since they're generic. Therefore, the problem might be way more complicated to fix considering I have no idea how to link to a slice or a pointer...

nixpulvis commented

Yea looking at the code, it seems things might stem from here:

rust/src/librustc_resolve/lib.rs

Lines 770 to 793 in 60960a2

impl PrimitiveTypeTable {
fn new() -> PrimitiveTypeTable {
let mut table = FxHashMap::default();
table.insert(sym::bool, Bool);
table.insert(sym::char, Char);
table.insert(sym::f32, Float(FloatTy::F32));
table.insert(sym::f64, Float(FloatTy::F64));
table.insert(sym::isize, Int(IntTy::Isize));
table.insert(sym::i8, Int(IntTy::I8));
table.insert(sym::i16, Int(IntTy::I16));
table.insert(sym::i32, Int(IntTy::I32));
table.insert(sym::i64, Int(IntTy::I64));
table.insert(sym::i128, Int(IntTy::I128));
table.insert(sym::str, Str);
table.insert(sym::usize, Uint(UintTy::Usize));
table.insert(sym::u8, Uint(UintTy::U8));
table.insert(sym::u16, Uint(UintTy::U16));
table.insert(sym::u32, Uint(UintTy::U32));
table.insert(sym::u64, Uint(UintTy::U64));
table.insert(sym::u128, Uint(UintTy::U128));
Self { primitive_types: table }
}
}

Based on this and the list of primatives in the STD docs, I'm guessing the following is a complete list of generic (or otherwise unimplemented) primitives:

  • array
  • fn
  • pointer
  • reference
  • slice
  • tuple
  • unit
  • never

This seems to be more broad than just generics however, since the () type isn't really generic. The issue seems to affect types with "names" different from their syntax, e.g. () vs unit.

Given the precedence set by the STD documentation for these primitives, I believe I'd expect to link to each with the English name and not the syntax, e.g. [pointer::is_null] and not [*::is_null] (or otherwise). This hasn't come up as an issue with codegen since as far as I can tell no primitives have methods defined on them without a self argument of some kind. This seems to be part of the need for the separate primitive modules.

I'm wondering if a solution to the need for separate primitive modules would also solve this. That's probably way more work than is needed for this specific issue however. Food for thought? Otherwise simply fixing this issue would be nice indeed, though does seem harder than I was hoping.

GuillaumeGomez commented

The solution would be to "simply" try to check the current path and if not working, try to check if the first part of the path can be considered as primitive type (such as slice or pointer) and retry with this change. We already do something similar in some cases.

  • 👍1
jyn514 commented

The solution would be to "simply" try to check the current path and if not working, try to check if the first part of the path can be considered as primitive type (such as slice or pointer) and retry with this change.

I'm not sure it's that simple. If you look at https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.PrimTy.html, none of the primitives you listed are present. Looking at https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/def/enum.Res.html, there isn't even a Res for all the types - ! and () are conspicuously absent.

The issue seems to affect types with "names" different from their syntax, e.g. () vs unit.

This isn't the issue, fn also doesn't work. The issue is that these are missing from PrimTy.

jyn514 commented

@rustbot modify labels: E-hard

jyn514 commented

A possible solution is to move from Res to Ty as the canonical way to look up primitives, using TyCtxt::type_of. Everything except primitives could keep using Res, it's only primitives that would have to change.

Relevant code:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Line 1237 in 121974e

fn is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> {

jyn514 commented

Yeah no I take it back, resolve() immediately returns the Res and so you can no longer distinguish it from ADTs. So this really does need a full refactor of collect_intra_doc_links.

jyn514 commented

Ok, the bits that are actually used from Res are:

  • For primitive types, just the URL fragment from is_primitive and the fact that it's a primitive
  • For ADTs, it calls register_res and stores that DefId. It also stores a URL fragment but I'm pretty sure that's just user defined and not tied to the Res.
    • register_res which mostly just calls
    inline::record_extern_fqn(cx, did, kind);
    if let TypeKind::Trait = kind {
        inline::record_extern_trait(cx, did);
    }

The URL fragment is independent of Res. The fact that it's a primitive is independent of Res. The Kind is somewhat tied to the Res, but you could get the same info from a Ty or TyKind. However, I don't see an easy way to go from an ADT to a TyKind or Ty.

So, this scheme might work:

enum Resolution {
    Def(DefKind, DefId),  // replaces `Res::Def`
    Primitive { fragment: String },
}

Hmm ... that doesn't seem so bad.

clarfonthey commented

Not sure if this is 100% related, but if you auto-link [()] or [!] it doesn't go to tuple or never as expected.

jyn514 commented

@clarfonthey correct, that's this issue.

  • 👍1