Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

rustdoc: Remove doc link resolution fallback to all macro_rules in the crate #96676

Merged
merged 1 commit into from
May 16, 2022
Merged

rustdoc: Remove doc link resolution fallback to all macro_rules in the crate #96676

merged 1 commit into from
May 16, 2022

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented May 3, 2022 •
edited
Loading

Fallback to all macro_rules in the crate is removed, as discussed in #96521, with compatibility measures described in #96676 (comment).

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label May 3, 2022
@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2022
@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented May 3, 2022

⌛ Trying commit 27f0349f6a579e74d1c1e2bade8ff093b7879135 with merge 6fd27704e643b25711e6a97ec2b04873b411b97d...

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2022
@bors
Copy link
Contributor

bors commented May 3, 2022

☀️ Try build successful - checks-actions
Build commit: 6fd27704e643b25711e6a97ec2b04873b411b97d (6fd27704e643b25711e6a97ec2b04873b411b97d)

@petrochenkov
Copy link
Contributor Author

@craterbot run mode=rustdoc

@craterbot
Copy link
Collaborator

👌 Experiment pr-96676 created and queued.
🤖 Automatically detected try build 6fd27704e643b25711e6a97ec2b04873b411b97d
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🚧 Experiment pr-96676 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-96676 is completed!
📊 323 regressed and 93 fixed (231017 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 11, 2022
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2022
@petrochenkov
Copy link
Contributor Author

There's a huge number of spurious regressions, but 190 regressions are indeed due to the removal of the macro_rules fallback.

I think I have a plan for addressing this:

  • First we remove the fallback from fn resolve_path
  • Then we detect this case in fn resolution_failure and add a note to the lint message telling that "a macro with this name exists in this crate, but it's not in scope at the link's location".
  • Then we quietly return a link pointing to the found macro_rules from fn resolution_failure.

As a result we only break crates that explicitly deny broken_intra_doc_links, and produce working links in other cases (e.g. for docs.rs) while at the same time pretending that such links are not produced by showing the broken_intra_doc_links warnings.

@petrochenkov
Copy link
Contributor Author

Marking this as blocked on #96345 because the plan is hard to implement without a preparational cleanup.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 13, 2022
petrochenkov
petrochenkov commented May 15, 2022
View reviewed changes
src/librustdoc/passes/collect_intra_doc_links.rs
@@ -1826,11 +1805,22 @@ fn resolution_failure(
diag.note(&note);
}

// If the link has `::` in it, assume it was meant to be an intra-doc link.
// Otherwise, the `[]` might be unrelated.
// FIXME: don't show this for autolinks (`<>`), `()` style links, or reference links
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The FIXME is no longer relevant after #96187 as I understand, so I removed it.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. Thanks for the cleanup!

@petrochenkov
Copy link
Contributor Author

PR updated in accordance with the plan from #96676 (comment).
@rustbot ready

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 15, 2022
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2022
@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented May 16, 2022

📌 Commit b4019de has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2022
@bors
Copy link
Contributor

bors commented May 16, 2022

⌛ Testing commit b4019de with merge c52b9c1...

@bors
Copy link
Contributor

bors commented May 16, 2022

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing c52b9c1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2022
@bors bors merged commit c52b9c1 into rust-lang:master May 16, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 16, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c52b9c1): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@davidbarsky davidbarsky mentioned this pull request May 18, 2022
Merged
@alygin alygin mentioned this pull request May 19, 2022
Merged
bors bot added a commit to crossbeam-rs/crossbeam that referenced this pull request May 19, 2022
@bors @cbeuw @alygin
Merge #831 #832
80224bc 
831: Add spin loop hints in tests for Miri r=taiki-e a=cbeuw

This is a better way to do #829 

Miri does not have a pre-emptive scheduler, so once the execution falls into a spin loop it'll hang forever: rust-lang/miri#1388

Similar measures (`yield_now()`) are already present in [some other tests](https://github.com/crossbeam-rs/crossbeam/blob/master/crossbeam-queue/tests/array_queue.rs), but it's missing here

832: Fix links to macro rules in docs r=taiki-e a=alygin

The fix provides explicit links to macro rules in docs because they [are not resolved globally anymore](rust-lang/rust#96676).


Co-authored-by: Andy Wang <cbeuw.andy@gmail.com>
Co-authored-by: Andrew Lygin <alygin@gmail.com>
@petrochenkov petrochenkov mentioned this pull request May 27, 2022
Closed
@hellow554 hellow554 mentioned this pull request Jun 2, 2022
Closed
@petrochenkov petrochenkov mentioned this pull request Jul 2, 2022
Closed
@jyn514 jyn514 mentioned this pull request Apr 27, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
7 participants
@petrochenkov @rust-highfive @bors @craterbot @GuillaumeGomez @rust-timer @rustbot