rustdoc: Regression in display of macro_rules! matchers

[camelid]

Oddly enough, the two lines (and the areas around them) that show up when I grep for "macro_rules! in rustdoc don't appear to have changed at all in a while, so I'm not sure what caused this regression. Perhaps the span of the matcher changed? I wasn't able to reproduce this locally, even when I tried using a cross-crate re-export with a copy-pasted version of core::todo (modulo replacing $crate:: with ::std::). Also, the bug doesn't appear with, e.g., panic!, but does with compile_error!. So, this seems to be a subtle bug.

$ rg '"macro_rules!' src/librustdoc/
src/librustdoc/clean/mod.rs
2167:                "macro_rules! {} {{\n{}}}",

src/librustdoc/clean/inline.rs
541:                "macro_rules! {} {{\n{}}}",

beta (incorrect)

stable (correct)

[camelid]

This bug doesn't occur for core::todo! or core::compile_error!, which makes me think it's a bug with cross-crate spans (either in rustc or rustdoc). I think std::panic! rendering normally is further evidence of this, because std::panic! is currently not a re-export of core::panic!.

[camelid]

I wonder if #80339 is related?

[jyn514]

@camelid you can test by bisecting

[jyn514]

That line of code is

rust/src/librustdoc/clean/inline.rs

Line 545 in 0a8629b

.map(|span| { format!(" {} => {{ ... }};\n", span.to_src(cx)) })

which hasn't changed in 2 years. So either span or to_src has changed.

[camelid]

@camelid you can test by bisecting

But I don't have an MCVE to bisect with. Can you bisect std itself?

[jyn514]

@camelid sure, it will just take a while 😅

[camelid]

I meant in terms of getting bisect-rustc to work with rustbuild so we're not also rebuilding rustdoc every time and instead are using CI rustdocs. Unless you're suggesting git bisect? But, yeah, it would probably take a while.

[jyn514]

@camelid you can use a custom script with bisect-rustc, it's a little tricky but it should work.

[camelid]

Ok, since this issue is present on beta but not stable, I think the regression occurred in 61edfd5...8a9fa36 (the merge-bases between master and stable/beta), so that narrows it down a bit.

[camelid]

I started scrolling through git log -S 61edfd591cedff66fca639c02f66984f6271e5a6...8a9fa3682dcf0de095ec308a29a7b19b0e011ef0 and found this commit that looks like it might be related: f94360f

[camelid]

Aha! I came up with a super simple repro:

pub use std::todo;

[camelid]

Unfortunately, this bug seems even weirder than I thought, because the bisection pointed to 2020-03-11 as the regression date, which is bizarre because the bug is not present on stable.

The commit range I got was 3dbade6...1581278.

The command I used was:

$ cargo bisect-rustc --prompt --start=2020-01-01 --end=2021-05-01 -- doc --open

[camelid]

Based on looking through the commit range, these are the two PRs that seem most likely to have caused the regression: #66364 and #69677. #66364 seems especially likely to me, as it changed macro definitions in metadata, which is more likely to cause cross-crate macro issues.

I still don't understand why this issue is not present on stable, given that it regressed more than a year ago.

[jyn514]

@camelid if you build from source with channel = stable, does it still reproduce the bug?

[camelid]

I asked about this regression on Zulip, and Eric Huss gave a very helpful response, explaining that the reason the bisection gave such weird results is because there was a regression in March 2020 that was then fixed in April 2020, but then regressed again recently, in April 2021. Eric was able to determine that the PR that most recently caused the regression was #84233, which I was able to confirm by re-bisecting.

searched nightlies: from nightly-2021-01-01 to nightly-2021-05-01
regressed nightly: nightly-2021-04-30
searched commits: from ca075d2 to 478a07d
regressed commit: 814a560

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-apple-darwin
Reproduce with:

cargo bisect-rustc --prompt --start=2021-01-01 --end=2021-05-01 --with-src -- doc --open 

[camelid]

Just on a hunch, 39648ea seems likely to be the commit that caused the regression. It also explains why I was not able to minimize the regression without using std, since it seems that the regression is in the handling of the rust-src component.

[jyn514]

I think the more interesting question is why rustdoc depends on having the source available, that seems like a bug.

[camelid]

Yeah, I was thinking about that. We should probably switch to pretty-printing the matchers.

[camelid]

Assigning priority as discussed in the prioritization working group.

@rustbot label: +P-high -I-prioritize

[camelid]

Yeah, I was thinking about that. We should probably switch to pretty-printing the matchers.

I think this change would make sense independently of this bug, for the reason mentioned in #86208 (comment), but the one issue is that it won't fix the underlying regression that this bug is a symptom of.

[jyn514]

@camelid what underlying regression? Nothing in the compiler should depend on if the source of dependencies is available IMO

[camelid]

@camelid what underlying regression? Nothing in the compiler should depend on if the source of dependencies is available IMO

Are there no other places where we use ToSource::to_src (a wrapper around span_to_snippet) in rustdoc?

EDIT: I looked and it seems like everywhere else handles no source gracefully.

[camelid]

I'm currently trying out a fix that switches us to using rustc_ast_pretty.

[camelid]

It's not pretty, but it does work: