Normalize `<X as Y>::T` for rustdoc #77467

jyn514 · 2020-10-02T20:38:22Z

Only run for QPath::Resolved with Some self parameter (<X as Y>::T)
Fall back to the previous behavior if the path can't be resolved

The first commit is a pure refactor and should probably be reviewed by @GuillaumeGomez. I recommend reviewing the second commit on its own.

Fixes #77459.

r? @eddyb
cc @danielhenrymantilla , @lcnr

jyn514 · 2020-10-02T20:59:20Z

Look at that beauty 🎉

for context, it looked like this before:

GuillaumeGomez

The generated doc is simpler, so I strongly approve this change! Well done. :)

danielhenrymantilla · 2020-10-02T21:19:35Z

Great job, @jyn514 !! 🔥

jyn514 · 2020-10-06T16:37:44Z

Summarizing my conversation with eddyb:

Putting this in hir::QPath seems wrong, it will break for cross-crate re-exports. It should go in ty::Projection instead.

The ParamEnv for this seems wrong - eddyb thinks that rustdoc needs to track the environment as it goes, the way other lints work:

rust/compiler/rustc_lint/src/late.rs

Lines 68 to 77 in 5849a7e

    
           fn with_param_env<F>(&mut self, id: hir::HirId, f: F) 
        
           where 
        
               F: FnOnce(&mut Self), 
        
           { 
        
               let old_param_env = self.context.param_env; 
        
               self.context.param_env = 
        
                   self.context.tcx.param_env(self.context.tcx.hir().local_def_id(id)); 
        
               f(self); 
        
               self.context.param_env = old_param_env; 
        
           }

However I think currently it won't cause ICEs because rustdoc never looks at the type after it's been normalized; clean() isn't going through tcx.

jyn514 · 2020-10-16T01:27:13Z

This now works for cross-crate re-exports, but does not work for 'partial' normalization like

<Vec<T> as IntoIterator>::IntoIter -> vec::IntoIter<T>

Fixing partial normalization requires actually having a correct ParamEnv, which requires a lot of infrastructure in rustdoc I'm not sure how to write.

@eddyb I know this isn't 100% correct, but do you think it's better than the current situation of no normalization at all? I'm not sure where to start for having a correct ParamEnv :/

jyn514 · 2020-10-19T12:12:32Z

Current status: this is blocked on #78082 to get the param context, since otherwise the API is different enough from rustc I wouldn't be convinced it had consistent behavior.

jyn514 · 2020-11-23T04:40:37Z

This is finally (finally) ready to go 🎉 🎉 🎉

It does need to wait for #79335 and all related PRs to land first, I'll bug Guillaume about those tomorrow. It does not need #78082.

@eddyb the relevant commits are 9b5f5e0 and 2833896.

jyn514 · 2020-11-23T04:56:27Z

Some more choice screenshots:

Before:

After:

GuillaumeGomez · 2020-11-24T13:19:24Z

Still looks good to me! Waiting for @eddyb now. :)

This uses the same `with_param_env` pattern that late lints use. Thanks to all the doctree refactors, this was very easy to add.

oli-obk · 2020-11-26T10:13:57Z

r? @oli-obk

oli-obk

just a nit, then this lgtm

src/librustdoc/clean/mod.rs

... and fix some fuzzy wording in the debug logging.

jyn514 · 2020-11-26T13:02:19Z

@bors r=oli-obk

🎉 🎉

bors · 2020-11-26T13:02:20Z

📌 Commit 277bdbc has been approved by oli-obk

bors · 2020-11-26T16:31:18Z

⌛ Testing commit 277bdbc with merge 65ecc48...

bors · 2020-11-26T18:51:38Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 65ecc48 to master...

Cleanup more of rustdoc - Use `Item::from_def_id` for StructField - Use `from_def_id_and_parts` for primitives and keywords - Take `String` instead of `Symbol` in `from_def_id` - this avoids having to intern then immediately stringify the existing string. - Remove unused `get_stability` and `get_deprecation` - Remove unused `attrs` field from `primitives` - Remove unused `attrs` field from `keywords` This will probably conflict with rust-lang#79335 and I would prefer for that PR to land first - I'm anxious for rust-lang#77467 to land :) Makes rust-lang#76998 easier to add. r? `@GuillaumeGomez`

…jyn514 Revert "Normalize `<X as Y>::T` for rustdoc" Reverts rust-lang#77467 by disabling normalization. See rust-lang#79459; I intend to reland normalization once that's fixed. r? `@Aaron1011` cc `@oli-obk` `@GuillaumeGomez`

Don't run `resolve_vars_if_possible` in `normalize_erasing_regions` Neither `@eddyb` nor I could figure out what this was for. I changed it to `assert_eq!(normalized_value, infcx.resolve_vars_if_possible(&normalized_value));` and it passed the UI test suite. <details><summary> Outdated, I figured out the issue - `needs_infer()` needs to come _after_ erasing the lifetimes </summary> Strangely, if I change it to `assert!(!normalized_value.needs_infer())` it panics almost immediately: ``` query stack during panic: #0 [normalize_generic_arg_after_erasing_regions] normalizing `<str::IsWhitespace as str::pattern::Pattern>::Searcher` #1 [needs_drop_raw] computing whether `str::iter::Split<str::IsWhitespace>` needs drop #2 [mir_built] building MIR for `str::<impl str>::split_whitespace` #3 [unsafety_check_result] unsafety-checking `str::<impl str>::split_whitespace` rust-lang#4 [mir_const] processing MIR for `str::<impl str>::split_whitespace` rust-lang#5 [mir_promoted] processing `str::<impl str>::split_whitespace` rust-lang#6 [mir_borrowck] borrow-checking `str::<impl str>::split_whitespace` rust-lang#7 [analysis] running analysis passes on this crate end of query stack ``` I'm not entirely sure what's going on - maybe the two disagree? </details> For context, this came up while reviewing rust-lang#77467 (cc `@lcnr).` Possibly this needs a crater run? r? `@nikomatsakis` cc `@matthewjasper`

jyn514 · 2020-12-29T21:07:11Z

Since #79469 and #79525, this now requires a feature gate, so removing relnotes.

jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-traits Area: Trait system labels Oct 2, 2020

rust-highfive assigned eddyb Oct 2, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2020

jyn514 force-pushed the query-docs branch 2 times, most recently from a1ad03f to 3a57b53 Compare October 2, 2020 20:58

GuillaumeGomez approved these changes Oct 2, 2020

View reviewed changes

jyn514 added the relnotes Marks issues that should be documented in the release notes of the next release. label Oct 3, 2020

jyn514 mentioned this pull request Oct 6, 2020

Don't run resolve_vars_if_possible in normalize_erasing_regions #77616

Merged

jyn514 force-pushed the query-docs branch 2 times, most recently from 0d06449 to c647bcb Compare October 16, 2020 01:09

jyn514 force-pushed the query-docs branch from 434db6b to 712eb3b Compare October 16, 2020 01:29

jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2020

jyn514 mentioned this pull request Nov 10, 2020

extend const generics test suite #78916

Merged

jyn514 added the A-associated-items Area: Associated items such as associated types and consts. label Nov 12, 2020

jyn514 force-pushed the query-docs branch 2 times, most recently from 49e8029 to 2833896 Compare November 23, 2020 04:40

jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Nov 23, 2020

jyn514 mentioned this pull request Nov 24, 2020

Cleanup more of rustdoc #79372

Merged

This comment has been minimized.

Sign in to view

Track ParamEnvs properly

6278daa

This uses the same `with_param_env` pattern that late lints use. Thanks to all the doctree refactors, this was very easy to add.

jyn514 force-pushed the query-docs branch from 2833896 to 6278daa Compare November 24, 2020 19:10

rust-highfive assigned oli-obk and unassigned eddyb Nov 26, 2020

oli-obk reviewed Nov 26, 2020

View reviewed changes

src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved

Remove redundant lift_to_tcx

277bdbc

... and fix some fuzzy wording in the debug logging.

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 Nov 26, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 26, 2020

bors merged commit 65ecc48 into rust-lang:master Nov 26, 2020

rustbot added this to the 1.50.0 milestone Nov 26, 2020

jyn514 deleted the query-docs branch November 26, 2020 18:58

weiznich mentioned this pull request Nov 27, 2020

Rustdoc regression (overflow evaluating the requirement …) #79459

Closed

jyn514 restored the query-docs branch November 27, 2020 14:43

jyn514 mentioned this pull request Nov 27, 2020

Revert "Normalize <X as Y>::T for rustdoc" #79469

Merged

oli-obk added a commit that referenced this pull request Nov 27, 2020

Revert the effect of #77467 by disabling normalization in rustdoc

8b81bb8

jyn514 deleted the query-docs branch December 10, 2020 21:34

jyn514 removed the relnotes Marks issues that should be documented in the release notes of the next release. label Dec 29, 2020

jyn514 mentioned this pull request Mar 2, 2021

Tracking Issue for -Z normalize-docs #82692

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize `<X as Y>::T` for rustdoc #77467

Normalize `<X as Y>::T` for rustdoc #77467

jyn514 commented Oct 2, 2020 •

edited

Loading

jyn514 commented Oct 2, 2020 •

edited

Loading

GuillaumeGomez left a comment

danielhenrymantilla commented Oct 2, 2020

jyn514 commented Oct 6, 2020 •

edited

Loading

jyn514 commented Oct 16, 2020 •

edited

Loading

jyn514 commented Oct 19, 2020

jyn514 commented Nov 23, 2020 •

edited

Loading

jyn514 commented Nov 23, 2020

GuillaumeGomez commented Nov 24, 2020

This comment has been minimized.

oli-obk commented Nov 26, 2020

oli-obk left a comment

jyn514 commented Nov 26, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

jyn514 commented Dec 29, 2020

Normalize <X as Y>::T for rustdoc #77467

Normalize <X as Y>::T for rustdoc #77467

Conversation

jyn514 commented Oct 2, 2020 • edited Loading

jyn514 commented Oct 2, 2020 • edited Loading

GuillaumeGomez left a comment

Choose a reason for hiding this comment

danielhenrymantilla commented Oct 2, 2020

jyn514 commented Oct 6, 2020 • edited Loading

jyn514 commented Oct 16, 2020 • edited Loading

jyn514 commented Oct 19, 2020

jyn514 commented Nov 23, 2020 • edited Loading

jyn514 commented Nov 23, 2020

GuillaumeGomez commented Nov 24, 2020

This comment has been minimized.

oli-obk commented Nov 26, 2020

oli-obk left a comment

Choose a reason for hiding this comment

jyn514 commented Nov 26, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

jyn514 commented Dec 29, 2020

Normalize `<X as Y>::T` for rustdoc #77467

Normalize `<X as Y>::T` for rustdoc #77467

jyn514 commented Oct 2, 2020 •

edited

Loading

jyn514 commented Oct 2, 2020 •

edited

Loading

jyn514 commented Oct 6, 2020 •

edited

Loading

jyn514 commented Oct 16, 2020 •

edited

Loading

jyn514 commented Nov 23, 2020 •

edited

Loading