-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Correctly encode doc attribute metadata #149919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Correctly encode doc attribute metadata #149919
Conversation
|
Some changes occurred in compiler/rustc_hir/src/attrs |
|
Is this regression user observable? If so, it would be nice to add a test exercising it. |
|
Would be nice indeed. Gonna try to trigger a bug with it. |
|
So that should bring back the crate metadata "lost" in the previous PR, right? We could do a perf run :3 r=me with the test if it's possible to craft one |
c906f8d to
5c91a70
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
So the regression is actually observable but I can't reproduce it locally: the source links are not present in a lot of So this PR is a very annoying mix of "believe me it works" and "we'll see once this is merged and std is regenerated if it fixed the issue". But if someone is able to write a reproducer, I'd be EXTREMELY thankful as this is a very big issue. Anyway, PR should be ready. I implemented |
This comment has been minimized.
This comment has been minimized.
5c91a70 to
854f619
Compare
|
If I don't commit all changes, obviously things can't work... |
Would looking at the std docs of a try build reproduce the issue then? (CI or opt-dist may need to be checked to ensure that the docs will be built for a try build). As far as the current implementation goes, it’s not the prettiest but it seems our hands are somewhat tied due to preserving the possibly unexpected current encoding w/r/t doc inlining. In that sense I don’t mind it too much as it fixes the issue, it seems, but would also appreciate @fmease’s opinion. |
…on, r=Kobzol Generate macro expansion for rust compiler crates docs This enables the `--generate-macro-expansion` rustdoc flag, generating possibility to expand macros directly in source code pages (rust-lang#137229). Needed this new feature when I was working on rust-lang#149919 and I thought "why not enable it by default?". So here we go. Not too sure who to r? here so: r? `@kobzol`
|
I don't know if it's worth much, but if I take a look at PR CI generated doc (in the "Artifacts" section), I can see that the "Source" links are present.
|
|
Can you check for |
|
So seems like the fix is all good. Drives me nuts to be unable to have a local reproducer though... I thought reexports of deps would be enough but doesn't seem so. |


Follow-up of #149645.
The change I made was slightly wrong. Originally the check was:
So we were checking that there was no
doc(inline)attribute. This PR should fix it.r? @lqd