Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Paths in macro visibilities are not linkified #83000

Closed
camelid opened this issue Mar 11, 2021 · 1 comment · Fixed by #132302
Closed

Paths in macro visibilities are not linkified #83000

camelid opened this issue Mar 11, 2021 · 1 comment · Fixed by #132302
Assignees
@fmease
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) A-visibility Area: Visibility / privacy C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Mar 11, 2021

It looks like we don't use links in macro visibilities:

#![feature(decl_macro)]

pub mod inner1 {
    pub mod inner2 {
        pub mod inner3 {
            pub(in crate::inner1) macro foo() {}
        }
    }
}

image

@jyn514 thinks this might be a bug.

Originally posted by @camelid in #82815 (comment)
@camelid camelid mentioned this issue Mar 11, 2021
Closed
@camelid camelid added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) A-visibility Area: Visibility / privacy C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 11, 2021
@camelid
Copy link
Member Author

camelid commented Mar 11, 2021

Note: I haven't tested macro_rules! (I think pub macro_rules! landed recently) nor proc macros.

@camelid camelid mentioned this issue Apr 16, 2021
Merged
@fmease fmease self-assigned this Oct 29, 2024
@fmease fmease mentioned this issue Oct 29, 2024
Merged
@bors bors closed this as completed in 3376eda Nov 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 14, 2024
@rust-timer
Unrolled build for rust-lang#132302
a646603 
Rollup merge of rust-lang#132302 - fmease:rustdoc-better-vis-for-macro-decl, r=notriddle

rustdoc: Treat declarative macros more like other item kinds

Apparently at some time in the past we were unable to generate an href for the module path inside the visibility of decl macros 2.0 (`pub(in ...)`). As a result of this, a whole separate function was introduced specifically for printing the visibility of decl macros that didn't attempt to generate any links. The description of PR rust-lang#84074 states:

> 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.

I can no longer reproduce the original issue. Reusing the existing visibility rendering logic *seems* to work just fine (I couldn't come up with any counterexamples, though I invite you to prove me wrong).

* Fixes rust-lang#83000
* Fixes the visibility showing up "twice" in rustdoc-JSON output: Once as the `visibility` field, once baked into the source[^1]
* Fixes `#[doc(hidden)]` not getting rendered on doc(hidden) decl macros 2.0 under `--document-hiden-items` (for decl macros 1.2 the issue remains; I will address this separately when fixing rust-lang#132304).

---

<details><summary>Outdated Section</summary>

NOTE: The current version of this PR is committing a UI crime, I'd like to receive feedback on that. Maybe you have a satisfactory solution for how to remedy it. Namely, as you know we have two different ways of / modes for highlighting code with color:

1. Only highlighting links / item paths and avoiding to highlight tokens by kind like keywords (to reduce visual noise and maybe also artifact size). Used for item declarations(\*).
2. Highlighting tokens by kind. Used for code blocks written by the user.

(\*): With the notable exception being macro declarations! Well, since this PR reuses the same function for rendering the item visibility (which only makes sense), we have a clash of modes: We now use both ways of highlighting code for decl macros: №1 for the visibility, №2 for the rest. This awkward. See for yourself:

* On master: ![Screenshot 2024-10-29 at 03-37-48 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/22f0ab6e-9ba9-4c4e-8fb0-0741c91d360b)
* On this branch: ![Screenshot 2024-10-29 at 03-36-41 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b11d81a3-3e2e-43cb-a5b8-6773a3048732)

</details>

Furthermore, we now no longer syntax-highlight declarative macros (be it `macro_rules!` or `macro`) since that was inconsistent with the way we render all other item kinds. See (collapsed) *Outdated Section* above. See also rust-lang#132302 (comment).

| On master | On this branch |
|---|---|
| ![Screenshot 2024-11-13 at 16-12-46 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/cb3aeb42-a56d-4ced-80d9-f2694f369af1) | ![Screenshot 2024-11-13 at 16-13-22 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b73bee50-1b85-4862-afba-5ad471443ccc) |

[^1]: E.g., `"visibility":{"restricted":{"parent":1,"path":"::a"}},/*OMITTED*/,"inner":{"macro":"pub(in a) macro by_example_vis_named($foo:expr) {\n    ...\n}"}`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fmease fmease

Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) A-visibility Area: Visibility / privacy C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rustdoc: Treat declarative macros more like other item kinds fmease/rust
3 participants
@Manishearth @fmease @camelid