[intra-doc-links] primitives inconsistently point offline or online in std docs

[jnqnfe]

I noticed with an offline copy of the Rust 1.47 docs from the package in the Debian Sid repo, that there is an undesirable inconsistency in where links to primitives point to within the std documentation.

Take for instance the documentation for the std::ffi::CStr::to_string_lossy() method, which I just happened to notice it on. All links except the one for the str type point to the offline pages, while the one for str points to the online nightly page. However contrastingly the other link to str just above that one, in the return type, points to the offline page. Essentially all links to std types point offline, except those to primatives, which point online when within the doc comments but offline if in the type/function description.

It seems that the handling of primatives for intra-doc-link processing makes an assumption that links should always point to the online page, without making any exception for the std crate itself, unlike what is done otherwise.

[championshuttler]

@jonas-schievink, I would like to contribute to this issue , can you please help me getting started? Thanks

[jyn514]

I double checked and this issue is still present on rustc 1.50.0-nightly (349b3b324 2020-11-29). The relevant code is

rust/src/librustdoc/clean/types.rs

Lines 727 to 742 in a094ff9

	let url = match cache.extern_locations.get(krate) {
	Some(&(_, _, ExternalLocation::Local)) => {
	let depth = CURRENT_DEPTH.with(|l| l.get());
	"../".repeat(depth)
	}
	Some(&(_, _, ExternalLocation::Remote(ref s))) => s.to_string(),
	Some(&(_, _, ExternalLocation::Unknown)) | None => String::from(
	// NOTE: intentionally doesn't pass crate name to avoid having
	// different primitive links between crates
	if UnstableFeatures::from_environment(None).is_nightly_build() {
	"https://doc.rust-lang.org/nightly"
	} else {
	"https://doc.rust-lang.org"
	},
	),
	};

I tried unsuccessfully to debug what's going on here in #74077. @championshuttler feel free to pick back up on that if you're interested.

[ijackson]

I just stumbled across this. It's weird. In my local docs install the primitive links in the "crate level docs" for std, and in the "all items", are right and point to the local copy of std's docs. It seems this only triggers with an explicit primitive@?

The code in #79630 (comment) is strange. Why would it ever be right to hardcode the url deep in the code like that?

I think maybe this is here so that individual online docs links in places like docs.rs point to docs.rust-lang.org but why not do whatever is done to make things like std::result::Result work properly?

[jyn514]

I think maybe this is here so that individual online docs links in places like docs.rs point to docs.rust-lang.org but why not do whatever is done to make things like std::result::Result work properly?

I don't know. The same check for cache.external_locations is done for both primitives and Result, the hardcoded URL is just a fallback. Fixing this requires figuring out why external_locations isn't being hit only for primitives.

[jyn514]

I found the issue. links() explicitly skips the check to href() for primitives, which only leaves hard-coded URLs as the fallback:

rust/src/librustdoc/clean/types.rs

Lines 483 to 488 in d5a406b

	// FIXME(83083): using fragments as a side-channel for
	// primitive names is very unfortunate
	None => {
	let relative_to = &cx.current;
	if let Some(ref fragment) = *fragment {
	let url = match cx.cache().extern_locations.get(&self.def_id.krate()) {

In particular, the check for extern_locations.get(&self.def_id.krate) is completely bogus, that's the crate of the item being documented, not the primitive. I have some partial work on fixing it locally, see https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/doc.28primitive.29

[jyn514]

In particular, the check for extern_locations.get(&self.def_id.krate) is completely bogus, that's the crate of the item being documented, not the primitive

... which will never be in extern_locations because it's always the current crate 🤦