Skip to content

Fix links to extern types in rustdoc (fixes #78777) #79182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 21, 2020
Merged

Fix links to extern types in rustdoc (fixes #78777) #79182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
071d8b1
Fix rustdoc: Referencing methods on extern_types does not resolve #78…
lochsh Nov 15, 2020
d38dbcb
Improve error message when we try to get_type on something that does …
lochsh Nov 15, 2020
32cd4bc
Add test
lochsh Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,12 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}

fn get_type(&self, id: DefIndex, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
self.root.tables.ty.get(self, id).unwrap().decode((self, tcx))
self.root
.tables
.ty
.get(self, id)
.unwrap_or_else(|| panic!("Not a type: {:?}", id))
.decode((self, tcx))
}

fn get_stability(&self, id: DefIndex) -> Option<attr::Stability> {
Expand Down
9 changes: 8 additions & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
Res::PrimTy(prim) => Some(
self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str),
),
Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::TyAlias, did) => {
Res::Def(
DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::TyAlias
| DefKind::ForeignTy,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be interested to see if the following cause crashes:

  • OpaqueTy
  • AssocTy

(I got these from https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/def/enum.DefKind.html.)

AssocTy will definitely require larger refactors to fix properly (we'd have to call resolve_associated_trait_item recursively or something like that) but I think OpaqueTy may work without further changes. If so, please add a test case:

//! Link to [Opaque::Ty]
#![feature(type_alias_impl_trait)]

type Opaque = impl Trait;

pub trait Trait {
  type Ty;
}

impl Trait for () {
    type Ty = usize;
}

pub fn f() -> Opaque {
}

Copy link
Contributor Author

@lochsh lochsh Nov 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, I get the same behaviour for this test case whether DefKind::OpaqueTy is included in the match pattern or not:

warning: unresolved link to `Opaque::Ty`
 --> test.rs:1:14
  |
1 | //! Link to [Opaque::Ty]
  |              ^^^^^^^^^^ the type alias `Opaque` has no associated item named `Ty`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default

warning: 1 warning emitted

Here is the log output that seems relevant:

Opaque)))) in namespace TypeNS                                                                                                                                                                                                   [69/81]
DEBUG rustdoc::passes::collect_intra_doc_links looking for associated item named Ty for item DefId(0:3 ~ test[8787]::Opaque)
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Send", args: AngleBracketed { args: [], bindings: []
 } }] }, param_names: None, did: DefId(2:2047 ~ core[fe86]::marker::Send), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Sync", args: AngleBracketed { args: [], bindings: []
 } }] }, param_names: None, did: DefId(2:2059 ~ core[fe86]::marker::Sync), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Unpin", args: AngleBracketed { args: [], bindings: [
] } }] }, param_names: None, did: DefId(2:2088 ~ core[fe86]::marker::Unpin), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Freeze", args: AngleBracketed { args: [], bindings:
[] } }] }, param_names: None, did: DefId(2:2075 ~ core[fe86]::marker::Freeze), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "UnwindSafe", args: AngleBracketed { args: [], bindin
gs: [] } }] }, param_names: None, did: DefId(1:3925 ~ std[7f8a]::panic::UnwindSafe), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "RefUnwindSafe", args: AngleBracketed { args: [], bin
dings: [] } }] }, param_names: None, did: DefId(1:3926 ~ std[7f8a]::panic::RefUnwindSafe), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "From", args: AngleBracketed { args: [Type(Generic("T
"))], bindings: [] } }] }, param_names: None, did: DefId(2:1965 ~ core[fe86]::convert::From), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Into", args: AngleBracketed { args: [Type(Generic("U
"))], bindings: [] } }] }, param_names: None, did: DefId(2:1962 ~ core[fe86]::convert::Into), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Borrow", args: AngleBracketed { args: [Type(Generic(
"T"))], bindings: [] } }] }, param_names: None, did: DefId(2:1722 ~ core[fe86]::borrow::Borrow), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "BorrowMut", args: AngleBracketed { args: [Type(Gener
ic("T"))], bindings: [] } }] }, param_names: None, did: DefId(2:1725 ~ core[fe86]::borrow::BorrowMut), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "TryFrom", args: AngleBracketed { args: [Type(Generic
("U"))], bindings: [] } }] }, param_names: None, did: DefId(2:1972 ~ core[fe86]::convert::TryFrom), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "TryInto", args: AngleBracketed { args: [Type(Generic
("U"))], bindings: [] } }] }, param_names: None, did: DefId(2:1968 ~ core[fe86]::convert::TryInto), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering auto or blanket impl for trait Some(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Any", args: AngleBracketed { args: [], bindings: []
} }] }, param_names: None, did: DefId(2:2576 ~ core[fe86]::any::Any), is_generic: false })
DEBUG rustdoc::passes::collect_intra_doc_links considering traits {}
DEBUG rustdoc::passes::collect_intra_doc_links the candidates were []
DEBUG rustdoc::passes::collect_intra_doc_links got associated item kind None
DEBUG rustdoc::passes::collect_intra_doc_links Opaque::Ty resolved to Err(()) in namespace ValueNS
DEBUG rustdoc::passes::collect_intra_doc_links Opaque resolved to Ok((Path { span: test.rs:1:1: 1:1 (#0), segments: [PathSegment { ident: Opaque#0, id: NodeId(38), args: None }], tokens: None }, Def(TyAlias, DefId(0:3 ~ test[8787]::
Opaque)))) in namespace TypeNS
DEBUG rustdoc::passes::collect_intra_doc_links looking for associated item named Ty for item DefId(0:3 ~ test[8787]::Opaque)
DEBUG rustdoc::passes::collect_intra_doc_links considering traits {}
DEBUG rustdoc::passes::collect_intra_doc_links the candidates were []
DEBUG rustdoc::passes::collect_intra_doc_links got associated item kind None
DEBUG rustdoc::passes::collect_intra_doc_links looking for variants or fields named Ty for DefId(0:3 ~ test[8787]::Opaque)
DEBUG rustdoc::passes::collect_intra_doc_links looking for enum variant Opaque::Ty
DEBUG rustdoc::passes::collect_intra_doc_links Opaque resolved to Ok((Path { span: test.rs:1:1: 1:1 (#0), segments: [PathSegment { ident: Opaque#0, id: NodeId(39), args: None }], tokens: None }, Def(TyAlias, DefId(0:3 ~ test[8787]::
Opaque)))) in namespace TypeNS
DEBUG rustdoc::passes::collect_intra_doc_links found partial_res=Def(TyAlias, DefId(0:3 ~ test[8787]::Opaque))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ok. The bug here is that considering traits {} should have Trait, but we can fix it later, I'm not sure this is actually something rustdoc should do since impl Trait is supposed to hide which type is being returned.

did,
) => {
debug!("looking for associated item named {} for item {:?}", item_name, did);
// Checks if item_name belongs to `impl SomeItem`
let assoc_item = cx
Expand Down
18 changes: 18 additions & 0 deletions src/test/rustdoc/intra-link-extern-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![feature(extern_types)]

extern {
pub type ExternType;
}

impl ExternType {
pub fn f(&self) {

}
}

// @has 'intra_link_extern_type/foreigntype.ExternType.html'
// @has 'intra_link_extern_type/fn.links_to_extern_type.html' \
// 'href="../intra_link_extern_type/foreigntype.ExternType.html#method.f"'
/// See also [ExternType::f]
pub fn links_to_extern_type() {
}