Skip to content

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

Merged
bors merged 2 commits intorust-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 intorust-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.

@rustbot rustbot 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-highfive rust-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
Copy link
Copy Markdown
Collaborator

bors commented Jul 25, 2022

📌 Commit 42a4419 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors 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
Copy link
Copy Markdown
Member

jyn514 commented Jul 25, 2022

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) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut node: hir::HirId) -> bool {
while let Some(id) = tcx.hir().get_enclosing_scope(node) {
node = id;
if tcx.hir().attrs(node).lists(sym::doc).has_word(sym::hidden) {
return true;
}
}
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.
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:master Jul 27, 2022
@rustbot rustbot 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
Contributor Author

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

Fixes rust-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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant 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