rustdoc: Resolve doc links referring to macro_rules items

[petrochenkov]

cc #81633

UPD: the fallback to considering all macro_rules in the crate for unresolved names is not removed in this PR, it will be removed separately and will be run through crater.

[rust-highfive]

r? @notriddle

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

[notriddle]

Overall, I like this change. Using the "real" name resolver, instead of dumping all the macro_rules into a big global hash map, is definitely a bug fix. The rustdoc name resolver should act like the regular one as much as is reasonable.

But (1) this is a breaking change, and (2) even though I was assigned to it, I'm still not really the best expert on this kind of change.

[notriddle]

Changes like this imply the need for a Crater run and rel-notes tag. Right?

[petrochenkov]

Only removal of the fn resolve_macro_rules fallback can cause a noticeable breakage here.
I can move it to a separate PR so it doesn't block landing the remaining changes here (I plan to do some more work on top of them).

[notriddle]

Only removal of the fn resolve_macro_rules fallback can cause a noticeable breakage here. I can move it to a separate PR so it doesn't block landing the remaining changes here (I plan to do some more work on top of them).

That sounds great. I actually do want to land this change. It's just that we know there's going to be breakage, and we know docs.rs doesn't make warnings very visible, so anyone publishing there might not even notice.

[petrochenkov]

we know docs.rs doesn't make warnings very visible, so anyone publishing there might not even notice

Then crater won't catch them either?
Only crates with deny(rustdoc::broken_intra_doc_links) will regress in both cases.

[notriddle]

That's a good point. 😐

Is there any clear way to identify or mitigate the breakage here? I'd like to use a backcompat warning, but anyone using docs.rs won't actually see it. I thought maybe a crater run could be made with deny(rustdoc::broken_intra_doc_links) temporarily turned on by default, but that will probably surface a bunch of false positives.

Maybe a crater run with a tweaked version that, instead of making the fallback fill in a resolution, the "fallback" is tweaked to produce a hard error whenever it actually runs and resolves something?

[petrochenkov]

Maybe a crater run with a tweaked version that, instead of making the fallback fill in a resolution, the "fallback" is tweaked to produce a hard error whenever it actually runs and resolves something?

Ok, that should be possible.

I'd like to use a backcompat warning, but anyone using docs.rs won't actually see it.

rustdoc::broken_intra_doc_links is already a warning, so reporting another warning saying that we are going to report this warning is a bit strange.

[petrochenkov]

Updated the PR, removal of the fn resolve_macro_rules fallback is reverted now.
@rustbot ready

[GuillaumeGomez]

So unless I missed something, we need a crater run here, right?

[petrochenkov]

No, crater run is no longer needed.
It will be needed in the next PR that will attempt to remove the resolve_macro_rules fallback.

[GuillaumeGomez]

Then let's go. Thanks!

@bors: r=notriddle,GuillaumeGomez

[bors]

📌 Commit 6083db7 has been approved by notriddle,GuillaumeGomez

[bors]

⌛ Testing commit 6083db7 with merge 4dd8b42...

[bors]

☀️ Test successful - checks-actions
Approved by: notriddle,GuillaumeGomez
Pushing 4dd8b42 to master...

[rust-timer]

Finished benchmarking commit (4dd8b42): 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