Skip to content

rustdoc: fix ICE when delegating functions with impl Trait as argument type#156498

Open
cijiugechu wants to merge 1 commit into
rust-lang:mainfrom
cijiugechu:fn_delegation-rustdoc
Open

rustdoc: fix ICE when delegating functions with impl Trait as argument type#156498
cijiugechu wants to merge 1 commit into
rust-lang:mainfrom
cijiugechu:fn_delegation-rustdoc

Conversation

@cijiugechu

@cijiugechu cijiugechu commented May 12, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

HIR signatures for delegation items contain InferDelegation placeholders, clean the inherited fn_sig signature instead.

Closes #155728

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels May 12, 2026
@rustbot

rustbot commented May 12, 2026

Copy link
Copy Markdown
Collaborator

r? @lolbinarycat

rustbot has assigned @lolbinarycat.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: rustdoc
  • rustdoc expanded to 9 candidates
  • Random selection from GuillaumeGomez, camelid, fmease, lolbinarycat, notriddle

@cijiugechu cijiugechu added the F-fn_delegation `#![feature(fn_delegation)]` label May 12, 2026
cijiugechu
cijiugechu commented May 12, 2026
View reviewed changes
Comment thread src/librustdoc/clean/simplify.rs
@lolbinarycat lolbinarycat changed the title rustdoc: fix delegated impl signatures rustdoc: fix ICE when delegating functions with impl Trait as argument type May 12, 2026
lolbinarycat
lolbinarycat requested changes May 12, 2026
View reviewed changes
Comment thread tests/rustdoc-html/fn-delegation-impl-trait.rs
Comment thread tests/rustdoc-html/fn-delegation-impl-trait.rs Outdated
Comment thread tests/rustdoc-html/fn-delegation-impl-trait.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2026
@cijiugechu cijiugechu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 14, 2026
@lolbinarycat

lolbinarycat commented May 16, 2026

Copy link
Copy Markdown
Contributor

Looks good! If you could also squash your commits that would be great!

@cijiugechu cijiugechu force-pushed the fn_delegation-rustdoc branch from d9493f2 to 04da15e Compare May 17, 2026 03:50
@cijiugechu

cijiugechu commented May 17, 2026

Copy link
Copy Markdown
Member Author

Looks good! If you could also squash your commits that would be great!

Done, thanks!

@rust-bors

This comment has been minimized.

Sign in to view
@cijiugechu cijiugechu force-pushed the fn_delegation-rustdoc branch from 04da15e to 913165b Compare June 2, 2026 14:10
@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cijiugechu cijiugechu force-pushed the fn_delegation-rustdoc branch from 913165b to 58be455 Compare June 2, 2026 15:36
@rustbot

rustbot commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in GUI tests.

cc @GuillaumeGomez

lolbinarycat
lolbinarycat reviewed Jun 2, 2026
View reviewed changes

@lolbinarycat lolbinarycat left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found a few more issues, thanks for your patience.

View changes since this review

Comment thread src/librustdoc/clean/simplify.rs Outdated
// FIXME(sized-hierarchy): Always skip `MetaSized` bounds so that only `?Sized`
// is shown and none of the new sizedness traits leak into documentation.
false
bounds.retain(|b| !b.is_sized_bound(cx.tcx));

@lolbinarycat lolbinarycat Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like a fix for an unrelated bug, could also make a regression test for that bug?

@cijiugechu cijiugechu Jun 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the simplify.rs change after rebasing.

Comment thread tests/rustdoc-gui/go-to-collapsed-elem.goml Outdated

// We need to disable this check because `trait.impl/lib2/scroll_traits/trait.Iterator.js`
// doesn't exist.
fail-on-request-error: false

@lolbinarycat lolbinarycat Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does it not exist? is this a bug? if so it at least deserves a FIXME, if not a github issue.

@cijiugechu cijiugechu Jun 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this while resolving the previous conflict after seeing a CI error I hadn’t seen before. Since this PR didn’t seem related to that failure, I suspected it might be flaky CI and disabled the check temporarily. I removed this in latest commit

Comment thread tests/rustdoc-html/fn-delegation-impl-trait.rs Outdated
Comment thread src/librustdoc/clean/mod.rs Outdated
if !function.generics.where_predicates.contains(&pred) {
function.generics.where_predicates.push(pred);
}
}

@lolbinarycat lolbinarycat Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this code reorder generic parameters under certain situations? it would be problematic if it did, since the order of generic parameters is important when using turbofish syntax. Even if there's no situation where this can cause problems currently, substituting the placeholders in a single pass would likely be more future proof than removing then re-adding them.

@cijiugechu cijiugechu Jun 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that was a good point. I changed the delegation path

fmease
fmease reviewed Jun 2, 2026
View reviewed changes
Comment thread src/librustdoc/clean/simplify.rs Outdated

// FIXME(sized-hierarchy): Always skip `MetaSized` bounds so that only `?Sized`
// is shown and none of the new sizedness traits leak into documentation.
bounds.retain(|b| !b.is_meta_sized_bound(cx.tcx));

@fmease fmease Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can bounds.len() > 1 with this PR? In my open PR #157262 I actually make rustdoc crash here if it has more than one bound (I know I know that's also not great; ideally the data types would be better or rather ideally we would directly process middle::ty types here, cc #113015).

View changes since the review

@cijiugechu cijiugechu Jun 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After rebasing, I removed the simplify.rs change entirely.

@rust-bors

This comment has been minimized.

Sign in to view
@cijiugechu cijiugechu force-pushed the fn_delegation-rustdoc branch from 58be455 to af8418d Compare June 7, 2026 15:26
@rustbot

rustbot commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fmease fmease fmease left review comments

@lolbinarycat lolbinarycat lolbinarycat requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@lolbinarycat lolbinarycat

Labels

F-fn_delegation `#![feature(fn_delegation)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ICE]: rustdoc: assertion failed: cx.impl_trait_bounds.is_empty()

5 participants

@cijiugechu @rustbot @lolbinarycat @rust-log-analyzer @fmease