[intra-doc links] Don't check feature gates of items re-exported across crates

[jyn514]

It should be never break another crate to re-export a public item.

Note that this doesn't check the feature gate at
all for other crates:

• Feature-gates aren't currently serialized, so the only way to check
  the gate is with ad-hoc attribute checking.
• Checking the feature gate twice (once when documenting the original
  crate and one when documenting the current crate) seems not great.

This should still catch using the feature most of the time though, since
people tend to document their own crates.

Closes #82284.

r? @Manishearth

[Mark-Simulacrum]

I would expect us to check the features at definition time; if we're re-exporting from a crate then we can bypass the feature check IMO, rather than re-verifying.

[jyn514]

I would expect us to check the features at definition time; if we're re-exporting from a crate then we can bypass the feature check IMO, rather than re-verifying.

The problem is that this feature is checked in rustdoc, and rustdoc may not be run on the original crate. Consider a proc-macro that's re-exported into a facade crate, where only the facade is documented.

[jyn514]

Checking this in rustc instead of rustdoc would require moving intra-doc link parsing into rustc proper, which has other problems: #79542

[jyn514]

Consider a proc-macro that's re-exported into a facade crate, where only the facade is documented.

Well, this isn't a very good example because rustdoc has to document the proc-macro, it won't be excluded by --no-deps. A better example is someone writing documentation but never testing it locally; cargo publish doesn't run rustdoc. Then anyone can re-export their items to bypass the feature gate.

[Mark-Simulacrum]

It seems like ultimately this doesn't help much then, because the feature gate can get omitted in the upstream crate (and not noticed as being actually required), and that causes downstream breakage during documentation building. Can we just leave the "loophole" open perhaps, and not check not-local definitions for using enabled features?

[jyn514]

It seems like ultimately this doesn't help much then, because the feature gate can get omitted in the upstream crate (and not noticed as being actually required), and that causes downstream breakage during documentation building.

The difference is that there's something the upstream crate can do to fix the breakage (enable the feature gate). Right now the upstream crate can't do anything and each individual crate re-exporting has to enable the feature gate themselves.

Concretely, this means since std has #![feature(intra_doc_pointers)], serde will stop breaking when you try to document it.

[jyn514]

This now checks the feature-gates of the crate where the item was defined, instead of the crate where it was re-exported.

I reverted this, now it only checks items in the local crate.

[camelid]

I believe Manish is on break to work on Vaccinate CA, so you probably want to re-assign this PR.

[jyn514]

@camelid do you have time to review?

[camelid]

Probably not soon, and also this bug seems too tricky for me to feel comfortable reviewing a fix for.

[Manishearth]

I'm not "on break" i'm just slower to do reviews, and prefer to not take on large tasks. This is probably okay

[Manishearth]

@bors r+

[bors]

📌 Commit fdb32e9 has been approved by Manishearth

[jyn514]

Seems spurious:

2021-02-21T03:46:15.1791994Z ##[section]Starting: Request a runner to run this job
2021-02-21T03:46:20.8597228Z Can't find any online and idle self-hosted runner in current repository that matches the required labels: 'macos-latest'
2021-02-21T03:46:20.9129134Z Can't find any online and idle self-hosted runner in current repository's organization account that matches the required labels: 'macos-latest'
2021-02-21T03:46:21.0693717Z Found online and idle hosted runner in current repository's organization account that matches the required labels: 'macos-latest'
2021-02-21T03:46:21.1705953Z ##[section]Finishing: Request a runner to run this job

@bors retry

[bors]

⌛ Testing commit fdb32e9 with merge c9905c59729e4ca0b67db0917344fea83c368cff...

[bors]

💔 Test failed - checks-actions

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[jyn514]

2021-02-21T19:34:55.6912480Z ##[section]Starting: Request a runner to run this job
2021-02-21T19:34:56.0401064Z Can't find any online and idle self-hosted runner in current repository that matches the required labels: 'ubuntu-latest'
2021-02-21T19:34:56.0902101Z Can't find any online and idle self-hosted runner in current repository's organization account that matches the required labels: 'ubuntu-latest'
2021-02-21T19:34:56.2419675Z Found online and idle hosted runner in current repository's organization account that matches the required labels: 'ubuntu-latest'
2021-02-21T19:34:56.4622354Z ##[section]Finishing: Request a runner to run this job

😕 @rust-lang/infra do you know why this keeps failing? It's happened twice now.

@bors retry

[bors]

⌛ Testing commit fdb32e9 with merge 24bfcee...

[Dylan-DPC-zz]

@bors rollup=never

[bors]

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing 24bfcee to master...

[jyn514]

Note: although this landed in 1.52 and #81015 landed in 1.51, it doesn't need to be backported because the first time this syntax of link was in libstd was in #81172, which also landed in 1.52.

[jyn514]

Well, but I guess this is bad in general in case people use it in their own libraries.

@rustbot label: +beta-nominated

[camelid]

Backport approval checklist:

• @jyn514
• @camelid
• @GuillaumeGomez
• [mystery person]

[camelid]

cc @rust-lang/rustdoc for backport decision