Skip to content

Prefer visibility map parents that are not doc(hidden) first - #99698

Merged
bors merged 2 commits into
rust-lang:masterfrom
compiler-errors:no-doc-hidden
Jul 27, 2022
Merged

Prefer visibility map parents that are not doc(hidden) first#99698
bors merged 2 commits into
rust-lang:masterfrom
compiler-errors:no-doc-hidden

Conversation

@compiler-errors

Copy link
Copy Markdown
Contributor

Far simpler approach to #98876.

This only fixes the case where the parent is doc(hidden), not where the child is doc(hidden) since I don't know how to get the attrs on the import statement given a ModChild... I'll try to follow up with that, but this is a good first step.

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 24, 2022
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @cjgillot

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2022
@cjgillot

Copy link
Copy Markdown
Contributor

The best way to access the attributes on a ModChild would probably to add a DefId to the ModChild.
In the mean time, this implementation looks good enough.
@bors r+

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 42a4419 has been approved by cjgillot

It is now in the queue for this repository.

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2022
@jyn514

jyn514 commented Jul 25, 2022

Copy link
Copy Markdown
Member

This only fixes the case where the parent is doc(hidden), not where the child is doc(hidden) since I don't know how to get the attrs on the import statement given a ModChild... I'll try to follow up with that, but this is a good first step.

not sure how hard this is in rustc_metadata, but rustdoc does it like so:

pub(crate)fninherits_doc_hidden(tcx:TyCtxt<'_>,mutnode: hir::HirId) -> bool{
whileletSome(id) = tcx.hir().get_enclosing_scope(node){
node = id;
if tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden){
returntrue;
}
}
false
}

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 26, 2022
…gillot
Prefer visibility map parents that are not `doc(hidden)` first
Far simpler approach to rust-lang#98876.
This only fixes the case where the parent is `doc(hidden)`, not where the child is `doc(hidden)` since I don't know how to get the attrs on the import statement given a `ModChild`... I'll try to follow up with that, but this is a good first step.
@JohnTitorJohnTitor mentioned this pull request Jul 27, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2022
Rollup of 8 pull requests
Successful merges:
- rust-lang#98583 (Stabilize Windows `FileTypeExt` with `is_symlink_dir` and `is_symlink_file`)
- rust-lang#99698 (Prefer visibility map parents that are not `doc(hidden)` first)
- rust-lang#99700 (Add a clickable link to the layout section)
- rust-lang#99712 (passes: port more of `check_attr` module)
- rust-lang#99759 (Remove dead code from cg_llvm)
- rust-lang#99765 (Don't build std for *-uefi targets)
- rust-lang#99771 (Update pulldown-cmark version to 0.9.2 (fixes url encoding for some chars))
- rust-lang#99775 (rustdoc: do not allocate String when writing path full name)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 3ca1c31 into rust-lang:masterJul 27, 2022
@rustbotrustbot added this to the 1.64.0 milestone Jul 27, 2022
@pnkfelix

Copy link
Copy Markdown
Contributor

@compiler-errors any chance that this contributed to the regression reported in #99792 (comment) ?

@compiler-errors

Copy link
Copy Markdown
ContributorAuthor

Yes very likely.

@compiler-errors
compiler-errors deleted the no-doc-hidden branch August 11, 2023 20:10
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 12, 2026
…-diagnostic-path, r=jackh726
Deprioritize doc(hidden) re-exports in diagnostic paths
Fixesrust-lang#153477.
This is the other half of rust-lang#99698, which fixed the case where the *parent module* is `#[doc(hidden)]` but left the case where the re-export itself is `#[doc(hidden)]` as a FIXME (with a tracking test in `dont-suggest-doc-hidden-variant-for-enum/hidden-child.rs`).
The problem: when a crate does `#[doc(hidden)] pub use core::error::Error`, diagnostics pick up the hidden re-export path instead of the canonical one. For example, `snafu::Error` instead of `std::error::Error`.
Two changes:
In `visible_parent_map`, the `add_child` closure now checks whether the re-export itself is `#[doc(hidden)]` via `reexport_chain` and sends it to `fallback_map`, same treatment as doc-hidden parents and underscore re-exports.
`should_encode_attrs` now returns `true` for `DefKind::Use`. Without this, `#[doc(hidden)]` on `use` items was never written to crate metadata, so `is_doc_hidden` always returned `false` cross-crate. This was the actual root cause, the check in `visible_parent_map` alone isn't enough if the attribute isn't in the metadata.
The existing FIXME test now serves as the regression test. The `.stderr` goes from suggesting `hidden_child::__private::Some(1i32)` to just `Some(1i32)`.
cc @eggyal
rust-timer added a commit that referenced this pull request Apr 12, 2026
Rollup merge of #153630 - arferreira:fix-doc-hidden-reexport-diagnostic-path, r=jackh726
Deprioritize doc(hidden) re-exports in diagnostic paths
Fixes#153477.
This is the other half of #99698, which fixed the case where the *parent module* is `#[doc(hidden)]` but left the case where the re-export itself is `#[doc(hidden)]` as a FIXME (with a tracking test in `dont-suggest-doc-hidden-variant-for-enum/hidden-child.rs`).
The problem: when a crate does `#[doc(hidden)] pub use core::error::Error`, diagnostics pick up the hidden re-export path instead of the canonical one. For example, `snafu::Error` instead of `std::error::Error`.
Two changes:
In `visible_parent_map`, the `add_child` closure now checks whether the re-export itself is `#[doc(hidden)]` via `reexport_chain` and sends it to `fallback_map`, same treatment as doc-hidden parents and underscore re-exports.
`should_encode_attrs` now returns `true` for `DefKind::Use`. Without this, `#[doc(hidden)]` on `use` items was never written to crate metadata, so `is_doc_hidden` always returned `false` cross-crate. This was the actual root cause, the check in `visible_parent_map` alone isn't enough if the attribute isn't in the metadata.
The existing FIXME test now serves as the regression test. The `.stderr` goes from suggesting `hidden_child::__private::Some(1i32)` to just `Some(1i32)`.
cc @eggyal
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@compiler-errors@rust-highfive@cjgillot@bors@jyn514@pnkfelix@rustbot@In-line