Skip to content

Commit

Permalink
Rollup merge of #75649 - jyn514:inherent-lang-impls, r=guillaumegomez
Browse files Browse the repository at this point in the history
Fix intra-doc links for inherent impls that are both lang items and not the default impl

I found in #75464 (comment) that `str::to_uppercase()` doesn't resolve while `str::trim()` does. The only real difference is that `to_uppercase` is defined in `alloc`, while trim is defined in `core`. It turns out that rustdoc was ignoring `lang_items.str_alloc_impl()` - it saw them in `collect_trait_impls`, but not for intra-doc links.

This uses the same `impls` for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?).

~~This needs a test case (probably just `str::to_uppercase`).~~ Added.

This is best reviewed commit-by-commit.

r? @GuillaumeGomez
  • Loading branch information
tmandry authored Aug 19, 2020
2 parents 4123237 + aff01f8 commit 7d14077
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 97 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4106,6 +4106,7 @@ dependencies = [
"rustc-rayon",
"serde",
"serde_json",
"smallvec 1.4.2",
"tempfile",
]

Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ minifier = "0.0.33"
rayon = { version = "0.3.0", package = "rustc-rayon" }
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
smallvec = "1.0"
tempfile = "3"
itertools = "0.8"
83 changes: 83 additions & 0 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::default::Default;
use std::fmt;
use std::hash::{Hash, Hasher};
use std::iter::FromIterator;
use std::lazy::SyncOnceCell as OnceCell;
use std::num::NonZeroU32;
use std::rc::Rc;
use std::sync::Arc;
Expand All @@ -19,12 +20,14 @@ use rustc_hir::lang_items;
use rustc_hir::Mutability;
use rustc_index::vec::IndexVec;
use rustc_middle::middle::stability;
use rustc_middle::ty::TyCtxt;
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::DUMMY_SP;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{self, FileName};
use rustc_target::abi::VariantIdx;
use rustc_target::spec::abi::Abi;
use smallvec::{smallvec, SmallVec};

use crate::clean::cfg::Cfg;
use crate::clean::external_path;
Expand Down Expand Up @@ -1264,6 +1267,86 @@ impl PrimitiveType {
}
}

pub fn impls(&self, tcx: TyCtxt<'_>) -> &'static SmallVec<[DefId; 4]> {
Self::all_impls(tcx).get(self).expect("missing impl for primitive type")
}

pub fn all_impls(tcx: TyCtxt<'_>) -> &'static FxHashMap<PrimitiveType, SmallVec<[DefId; 4]>> {
static CELL: OnceCell<FxHashMap<PrimitiveType, SmallVec<[DefId; 4]>>> = OnceCell::new();

CELL.get_or_init(move || {
use self::PrimitiveType::*;

/// A macro to create a FxHashMap.
///
/// Example:
///
/// ```
/// let letters = map!{"a" => "b", "c" => "d"};
/// ```
///
/// Trailing commas are allowed.
/// Commas between elements are required (even if the expression is a block).
macro_rules! map {
($( $key: expr => $val: expr ),* $(,)*) => {{
let mut map = ::rustc_data_structures::fx::FxHashMap::default();
$( map.insert($key, $val); )*
map
}}
}

let single = |a: Option<DefId>| a.into_iter().collect();
let both = |a: Option<DefId>, b: Option<DefId>| -> SmallVec<_> {
a.into_iter().chain(b).collect()
};

let lang_items = tcx.lang_items();
map! {
Isize => single(lang_items.isize_impl()),
I8 => single(lang_items.i8_impl()),
I16 => single(lang_items.i16_impl()),
I32 => single(lang_items.i32_impl()),
I64 => single(lang_items.i64_impl()),
I128 => single(lang_items.i128_impl()),
Usize => single(lang_items.usize_impl()),
U8 => single(lang_items.u8_impl()),
U16 => single(lang_items.u16_impl()),
U32 => single(lang_items.u32_impl()),
U64 => single(lang_items.u64_impl()),
U128 => single(lang_items.u128_impl()),
F32 => both(lang_items.f32_impl(), lang_items.f32_runtime_impl()),
F64 => both(lang_items.f64_impl(), lang_items.f64_runtime_impl()),
Char => single(lang_items.char_impl()),
Bool => single(lang_items.bool_impl()),
Str => both(lang_items.str_impl(), lang_items.str_alloc_impl()),
Slice => {
lang_items
.slice_impl()
.into_iter()
.chain(lang_items.slice_u8_impl())
.chain(lang_items.slice_alloc_impl())
.chain(lang_items.slice_u8_alloc_impl())
.collect()
},
Array => single(lang_items.array_impl()),
Tuple => smallvec![],
Unit => smallvec![],
RawPointer => {
lang_items
.const_ptr_impl()
.into_iter()
.chain(lang_items.mut_ptr_impl())
.chain(lang_items.const_slice_ptr_impl())
.chain(lang_items.mut_slice_ptr_impl())
.collect()
},
Reference => smallvec![],
Fn => smallvec![],
Never => smallvec![],
}
})
}

pub fn to_url_str(&self) -> &'static str {
self.as_str()
}
Expand Down
30 changes: 1 addition & 29 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ pub fn qpath_to_string(p: &hir::QPath<'_>) -> String {
}

pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut Vec<Item>) {
use self::PrimitiveType::*;
let tcx = cx.tcx;

for item in items {
Expand All @@ -370,34 +369,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V
None => continue,
},
};
let did = match primitive {
Isize => tcx.lang_items().isize_impl(),
I8 => tcx.lang_items().i8_impl(),
I16 => tcx.lang_items().i16_impl(),
I32 => tcx.lang_items().i32_impl(),
I64 => tcx.lang_items().i64_impl(),
I128 => tcx.lang_items().i128_impl(),
Usize => tcx.lang_items().usize_impl(),
U8 => tcx.lang_items().u8_impl(),
U16 => tcx.lang_items().u16_impl(),
U32 => tcx.lang_items().u32_impl(),
U64 => tcx.lang_items().u64_impl(),
U128 => tcx.lang_items().u128_impl(),
F32 => tcx.lang_items().f32_impl(),
F64 => tcx.lang_items().f64_impl(),
Char => tcx.lang_items().char_impl(),
Bool => tcx.lang_items().bool_impl(),
Str => tcx.lang_items().str_impl(),
Slice => tcx.lang_items().slice_impl(),
Array => tcx.lang_items().array_impl(),
Tuple => None,
Unit => None,
RawPointer => tcx.lang_items().const_ptr_impl(),
Reference => None,
Fn => None,
Never => None,
};
if let Some(did) = did {
for &did in primitive.impls(tcx) {
if !did.is_local() {
inline::build_impl(cx, did, None, ret);
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#![feature(ptr_offset_from)]
#![feature(crate_visibility_modifier)]
#![feature(never_type)]
#![feature(once_cell)]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
57 changes: 23 additions & 34 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::Ident;
use rustc_span::symbol::Symbol;
use rustc_span::DUMMY_SP;
use smallvec::SmallVec;

use std::cell::Cell;
use std::ops::Range;
Expand Down Expand Up @@ -270,18 +271,26 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
.ok_or(ErrorKind::ResolutionFailure)?;

if let Some((path, prim)) = is_primitive(&path, TypeNS) {
let did = primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)?;
return cx
.tcx
.associated_items(did)
.filter_by_name_unhygienic(item_name)
.next()
.and_then(|item| match item.kind {
ty::AssocKind::Fn => Some("method"),
_ => None,
})
.map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name))))
.ok_or(ErrorKind::ResolutionFailure);
for &impl_ in primitive_impl(cx, &path).ok_or(ErrorKind::ResolutionFailure)? {
let link = cx
.tcx
.associated_items(impl_)
.find_by_name_and_namespace(
cx.tcx,
Ident::with_dummy_span(item_name),
ns,
impl_,
)
.and_then(|item| match item.kind {
ty::AssocKind::Fn => Some("method"),
_ => None,
})
.map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name))));
if let Some(link) = link {
return Ok(link);
}
}
return Err(ErrorKind::ResolutionFailure);
}

let (_, ty_res) = cx
Expand Down Expand Up @@ -1238,26 +1247,6 @@ fn is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> {
}
}

fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option<DefId> {
let tcx = cx.tcx;
match path_str {
"u8" => tcx.lang_items().u8_impl(),
"u16" => tcx.lang_items().u16_impl(),
"u32" => tcx.lang_items().u32_impl(),
"u64" => tcx.lang_items().u64_impl(),
"u128" => tcx.lang_items().u128_impl(),
"usize" => tcx.lang_items().usize_impl(),
"i8" => tcx.lang_items().i8_impl(),
"i16" => tcx.lang_items().i16_impl(),
"i32" => tcx.lang_items().i32_impl(),
"i64" => tcx.lang_items().i64_impl(),
"i128" => tcx.lang_items().i128_impl(),
"isize" => tcx.lang_items().isize_impl(),
"f32" => tcx.lang_items().f32_impl(),
"f64" => tcx.lang_items().f64_impl(),
"str" => tcx.lang_items().str_impl(),
"bool" => tcx.lang_items().bool_impl(),
"char" => tcx.lang_items().char_impl(),
_ => None,
}
fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option<&'static SmallVec<[DefId; 4]>> {
Some(PrimitiveType::from_symbol(Symbol::intern(path_str))?.impls(cx.tcx))
}
35 changes: 1 addition & 34 deletions src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,40 +34,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate {
}

// Also try to inline primitive impls from other crates.
let lang_items = cx.tcx.lang_items();
let primitive_impls = [
lang_items.isize_impl(),
lang_items.i8_impl(),
lang_items.i16_impl(),
lang_items.i32_impl(),
lang_items.i64_impl(),
lang_items.i128_impl(),
lang_items.usize_impl(),
lang_items.u8_impl(),
lang_items.u16_impl(),
lang_items.u32_impl(),
lang_items.u64_impl(),
lang_items.u128_impl(),
lang_items.f32_impl(),
lang_items.f64_impl(),
lang_items.f32_runtime_impl(),
lang_items.f64_runtime_impl(),
lang_items.bool_impl(),
lang_items.char_impl(),
lang_items.str_impl(),
lang_items.array_impl(),
lang_items.slice_impl(),
lang_items.slice_u8_impl(),
lang_items.str_alloc_impl(),
lang_items.slice_alloc_impl(),
lang_items.slice_u8_alloc_impl(),
lang_items.const_ptr_impl(),
lang_items.mut_ptr_impl(),
lang_items.const_slice_ptr_impl(),
lang_items.mut_slice_ptr_impl(),
];

for def_id in primitive_impls.iter().filter_map(|&def_id| def_id) {
for &def_id in PrimitiveType::all_impls(cx.tcx).values().flatten() {
if !def_id.is_local() {
inline::build_impl(cx, def_id, None, &mut new_items);

Expand Down
32 changes: 32 additions & 0 deletions src/test/rustdoc/intra-link-primitive-non-default-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#![deny(broken_intra_doc_links)]

// ignore-tidy-linelength

// @has intra_link_primitive_non_default_impl/fn.str_methods.html
/// [`str::trim`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim"]' 'str::trim'
/// [`str::to_lowercase`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.to_lowercase"]' 'str::to_lowercase'
/// [`str::into_boxed_bytes`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.into_boxed_bytes"]' 'str::into_boxed_bytes'
/// [`str::replace`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html#method.replace"]' 'str::replace'
pub fn str_methods() {}

// @has intra_link_primitive_non_default_impl/fn.f32_methods.html
/// [f32::powi]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f32.html#method.powi"]' 'f32::powi'
/// [f32::sqrt]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f32.html#method.sqrt"]' 'f32::sqrt'
/// [f32::mul_add]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f32.html#method.mul_add"]' 'f32::mul_add'
pub fn f32_methods() {}

// @has intra_link_primitive_non_default_impl/fn.f64_methods.html
/// [`f64::powi`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f64.html#method.powi"]' 'f64::powi'
/// [`f64::sqrt`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f64.html#method.sqrt"]' 'f64::sqrt'
/// [`f64::mul_add`]
// @has - '//*[@href="https://doc.rust-lang.org/nightly/std/primitive.f64.html#method.mul_add"]' 'f64::mul_add'
pub fn f64_methods() {}

0 comments on commit 7d14077

Please sign in to comment.