Skip to content

Don't use unnormalized type in Ty::fn_sig call in rustdoc clean_middle_ty #102831

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 9, 2022
Merged

Don't use unnormalized type in Ty::fn_sig call in rustdoc clean_middle_ty #102831

merged 1 commit into from
Oct 9, 2022

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Oct 9, 2022 •
edited
Loading

Self-explanatory

Fixes #102828

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 9, 2022
@rust-highfive
Copy link
Contributor

r? @CraftSpider

(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 Oct 9, 2022
compiler-errors
compiler-errors commented Oct 9, 2022
View reviewed changes
src/librustdoc/clean/mod.rs
@@ -1582,12 +1582,12 @@ fn normalize<'tcx>(cx: &mut DocContext<'tcx>, ty: Ty<'_>) -> Option<Ty<'tcx>> {
}

pub(crate) fn clean_middle_ty<'tcx>(
this: Ty<'tcx>,
ty: Ty<'tcx>,
Copy link
Member Author

Choose a reason for hiding this comment

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

Intentionally renaming this -> ty so that the normalized let ty = ... below shadows the unnormalized type, so this doesn't happen again.

compiler-errors
compiler-errors commented Oct 9, 2022
View reviewed changes
src/librustdoc/clean/mod.rs
@@ -1610,7 +1610,6 @@ pub(crate) fn clean_middle_ty<'tcx>(
type_: Box::new(clean_middle_ty(ty, cx, None)),
},
ty::FnDef(..) | ty::FnPtr(_) => {
let ty = cx.tcx.lift(this).expect("FnPtr lift failed");
Copy link
Member Author

Choose a reason for hiding this comment

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

But technically this is the only fix needed -- this should've been cx.tcx.lift(ty), but also isn't needed at all.

@jyn514
Copy link
Member

jyn514 commented Oct 9, 2022

I didn't find this self explanatory at first - the problem here is that we were using a normalized and unnormalized type inconsistently, not just that it was unnormalized. The fix is to consistently use one or the other, and @compiler-errors chose the normalized version (which is what I'd prefer, thanks, otherwise -Znormalize-docs does nothing).

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 9, 2022

📌 Commit 7dedb91 has been approved by jyn514

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 Oct 9, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Oct 9, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 9, 2022
@Dylan-DPC
Rollup merge of rust-lang#102831 - compiler-errors:rustdoc-norm-oops,…
23c2d21 
… r=jyn514

Don't use unnormalized type in `Ty::fn_sig` call in rustdoc `clean_middle_ty`

Self-explanatory

Fixes rust-lang#102828
@JohnTitor JohnTitor mentioned this pull request Oct 9, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 9, 2022
@bors
Auto merge of rust-lang#102850 - JohnTitor:rollup-lze1w03, r=JohnTitor
81f3919 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101118 (fs::get_mode enable getting the data via fcntl/F_GETFL on major BSD)
 - rust-lang#102072 (Add `ptr::Alignment` type)
 - rust-lang#102799 (rustdoc: remove hover gap in file picker)
 - rust-lang#102820 (Show let-else suggestion on stable.)
 - rust-lang#102829 (rename `ImplItemKind::TyAlias` to `ImplItemKind::Type`)
 - rust-lang#102831 (Don't use unnormalized type in `Ty::fn_sig` call in rustdoc `clean_middle_ty`)
 - rust-lang#102834 (Remove unnecessary `lift`/`lift_to_tcx` calls from rustdoc)
 - rust-lang#102838 (remove cfg(bootstrap) from Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0a9b09f into rust-lang:master Oct 9, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 9, 2022
@compiler-errors compiler-errors deleted the rustdoc-norm-oops branch November 2, 2022 02:54
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@bors
Auto merge of rust-lang#102850 - JohnTitor:rollup-lze1w03, r=JohnTitor
d7638ed 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#101118 (fs::get_mode enable getting the data via fcntl/F_GETFL on major BSD)
 - rust-lang#102072 (Add `ptr::Alignment` type)
 - rust-lang#102799 (rustdoc: remove hover gap in file picker)
 - rust-lang#102820 (Show let-else suggestion on stable.)
 - rust-lang#102829 (rename `ImplItemKind::TyAlias` to `ImplItemKind::Type`)
 - rust-lang#102831 (Don't use unnormalized type in `Ty::fn_sig` call in rustdoc `clean_middle_ty`)
 - rust-lang#102834 (Remove unnecessary `lift`/`lift_to_tcx` calls from rustdoc)
 - rust-lang#102838 (remove cfg(bootstrap) from Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@CraftSpider CraftSpider

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.66.0
Development

Successfully merging this pull request may close these issues.

ICE: rustdoc: Ty::fn_sig() called on non-fn type, Znormalize-docs
6 participants
@compiler-errors @rust-highfive @jyn514 @bors @CraftSpider @rustbot