rustdoc: clean up and test macro visibility print #84074
Conversation
This fixes the overly-complex invariant mentioned in <rust-lang#83237 (comment)>, where the macro source can't have any links in it only because the cache hasn't been populated yet.
|
r? @CraftSpider (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
jyn514
added
the
T-rustdoc
| let first_name = | ||
| path.data[0].data.get_opt_name().expect("modules are always named"); | ||
| // modified from `resolved_path()` to work with `DefPathData` | ||
| let last_name = path.data.last().unwrap().data.get_opt_name().unwrap(); | ||
| let anchor = anchor(vis_did, &last_name.as_str(), cache).to_string(); | ||
|
|
||
| let mut s = "pub(".to_owned(); | ||
| if path.data.len() != 1 | ||
| || (first_name != kw::SelfLower && first_name != kw::Super) | ||
| { | ||
| s.push_str("in "); | ||
| } | ||
| let mut s = "pub(in ".to_owned(); |
There was a problem hiding this comment.
It seemed impossible for this conditional to evaluate to false. It's checking if the path is exactly equal to self or super, but there are already conditionals above that check if it's equal to the current module or the parent module.
Removing it also didn't break any test cases, supporting my dead-code theory.
jyn514
added
S-waiting-on-author
|
@notriddle I have a question about how this interacts with #83237 - you add an assert there that |
That assert is actually outdated. The |
|
I guess it no longer matters one way or the other, yeah - all the assert is checking is that to_source_with_space is consistent with the previous behavior, but the previous behavior was wrong if the cache wasn't empty. Removing the assert sounds fine, since the test suite passed with it once already, that gives me confidence it was at least correct most of the time. |
|
@bors r+ |
|
📌 Commit 2dfd0bf has been approved by |
bors
added
S-waiting-on-bors
… r=jyn514 rustdoc: clean up and test macro visibility print This fixes the overly-complex invariant mentioned in <rust-lang#83237 (comment)>, where the macro source can't have any links in it only because the cache hasn't been populated yet.
|
Will this fix #83000 or is it a pure cleanup with no (expected) behavior changes? |
|
It should have no visible behaviour changes. Just pure cleanup. |
|
☀️ Test successful - checks-actions |
This fixes the overly-complex invariant mentioned in #83237 (comment), where the macro source can't have any links in it only because the cache hasn't been populated yet.