rustdoc: Fix invalid handling of nested items with --document-private-items#110450
Conversation
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
I needed to move this nested enum outside because since it's not "processed" anymore, the failure doesn't appear anymore either.
There was a problem hiding this comment.
When this test case was first added in #75127, it seems like it was specifically meant to check for an ICE that only triggered when an item was declared inside of a function.
You can add a second enum if you want to keep the test triggering the error, but don't remove the one inside the function's body.
There was a problem hiding this comment.
It isn't triggered anymore. Hence why I moved it. Should I just remove the test then?
There was a problem hiding this comment.
The thing is, the test doesn't seem to exist to check the actual error (AFAICT, @jyn514 would know better). It seems to exist as a regression test for an ICE. infinite-recursive-type.rs already exists to test the case where an infinite recursive type is outside of a function.
If it's not generating an error, I think (if I'm wrong, please chime in @jyn514) this should be changed to a // check-pass test.
There was a problem hiding this comment.
Changing this to check-pass seems ok (moving the item to the top-level was definitely incorrect, thank you @notriddle for catching it). tbh I was hesitant to add this test originally, given that there are many other similar ICEs that we haven't bothered fixing in 3 years, but if in the future something changes we can revisit the decision.
While you're looking at this, could you please rename the test to tests/rustdoc-ui/error-in-impl-trait/infinite-recursive-type.rs so it's easier to recover the context? It sounds like this change was because of --document-private-items, so if you could also add a revision of the test where f and E are both public, that would be helpful too.
GuillaumeGomez
force-pushed
the
fix-nested-items-on-private-doc
branch
from
4b04c9d to
c456e15
Compare
|
I changed the ui-test to check that it passes (since it's supposed to check there is no ICE). |
|
@bors r=notriddle,jyn514 rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#109981 (Set commit information environment variables when building tools) - rust-lang#110348 (Add list of supported disambiguators and suffixes for intra-doc links in the rustdoc book) - rust-lang#110409 (Don't use `serde_json` to serialize a simple JSON object) - rust-lang#110442 (Avoid including dry run steps in the build metrics) - rust-lang#110450 (rustdoc: Fix invalid handling of nested items with `--document-private-items`) - rust-lang#110461 (Use `Item::expect_*` and `ImplItem::expect_*` more) - rust-lang#110465 (Assure everyone that `has_type_flags` is fast) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…sive-type, r=jyn514 Add a failing rustdoc-ui test for public infinite recursive type As suggested in rust-lang#110450 (comment). r? `@jyn514`
5 participants
Fixes #110422.
The problem is that only impl block and re-exported
macro_rules!items are "visible" as nested items. This PR adds the missing checks to handle this correctly.cc @compiler-errors
r? @notriddle