Recursively document methods via Deref traits

[jryans]

This changes rustdoc to recursively follow Deref targets so that methods from all levels are added to the rendered output. This implementation displays the methods from all levels in the expanded state with separate sections for each level.

Fixes #26207
Fixes #53038
Fixes #71640

r? @jyn514

[GuillaumeGomez]

That's really great! Thanks a lot for working on this.

[jyn514]

This is amazing ❤️ thank you so much for tackling it.

[jyn514]

What does target mean here? Could we get away without it?

Also, pretty-printing the type seems very unstable, since pretty-printing is unstable, and could have collisions if there are multiple types with the same name (but in different modules). I would rather have an opaque ID, the same way we do for impl blocks:
https://github.com/rust-lang/rust/blob/9accf1d281d676b81753892b193bcd20a49700c1/src/librustdoc/html/render/mod.rs#L3775

[GuillaumeGomez]

Also I just realized: I seem to recall us having a hashset to prevent IDs duplication. Maybe they should use it too. That could also allow to have shorter IDs.

[jryans]

target is just here as a prefix matching the text of section name, as in "Target = Foo". It could easily be removed as well, no strong opinion from my side.

I'll look into these suggestions for a more stable ID.

[jryans]

Okay, I removed the target prefix from the ID. I also changed to use the same derive_id uniqueness code as in other places. Since we need to access those same derived IDs in the sidebar rendering path, I added a new map to the Context to manage this.

[jyn514]

The impls were already documented by render_deref_methods and we don't want to duplicate them, is that the idea? The condition for running render_deref is different from what you check here though: this checks DerefFor and render_derf checks deref_impl.is_some(). Is there any time those could be different? If not, could you add an assert?

[jryans]

There are two concepts at play here simultaneously which is a bit confusing, so I'll try to explain and then we can work out whatever comments or assertions might be helpful.

Let's consider a two level Deref case:

pub struct Foo(Bar);
pub struct Bar(Baz);
pub struct Baz;

impl Deref for Foo {
    type Target = Bar;
    fn deref(&self) -> &Bar { &self.0 }
}

impl Deref for Bar {
    type Target = Baz;
    fn deref(&self) -> &Baz { &self.0 }
}

The goal of if let Some(impl_) = deref_impl when calling render_deref_methods is to descend to the methods of the Deref target: e.g. from Foo to Bar and from Bar to Baz.

The goal of if let AssocItemRender::DerefFor { .. } = what is to restrict what we render when we have already descended at least one level: e.g. when we're at Bar and Baz, we only want to show their methods and nothing else.

Does that help at all? 😅

[jyn514]

Yes, that helps me understand why these are different, thanks :) I still have my original question, although maybe I didn't word it very well. This return ignores synthetic impls, which is correct (the Target could implement Sized but not the original, and vice-versa), but it also ignores blanket and concrete impls. Where are those handled? Is it in render_deref_items?

[jryans]

At the moment, this change is preserving the previous behaviour: trait, synthetic, and blanket impls are all skipped for Deref targets (e.g. Bar and Baz in the above example). The methods in the Deref target that we want to include are rendered by the top bit of render_assoc_items.

[jyn514]

Got it, thanks - I missed that this is only skipping trait impls, not inherent impls. I'm not sure that behavior is correct, but it doesn't need to be fixed here.

[jryans]

@GuillaumeGomez, @jyn514: Okay, I think I've addressed all the open questions, so this should be ready for another look.

[jryans]

@rustbot label -S-waiting-on-author +S-waiting-on-review

[jryans]

Oh wait, I forgot the one about section target IDs... 😅

@rustbot label +S-waiting-on-author -S-waiting-on-review

[jryans]

The section IDs have been addressed as well, so this is ready for another review.

@rustbot label -S-waiting-on-author +S-waiting-on-review

[rust-log-analyzer]

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

.................................................................................................... 9000/11240
.................................................................................................... 9100/11240
.................................................................................................... 9200/11240
....................................i......i........................................................ 9300/11240
...........................................................................iiiiii..iiiiii.i......... 9400/11240
.................................................................................................... 9600/11240
.................................................................................................... 9700/11240
.................................................................................................... 9800/11240
.................................................................................................... 9900/11240
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 27 tests
iiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.086 seconds
Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i..ii....i.i....ii..........iiii.........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.44s

 finished in 2.533 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
---- [rustdoc] rustdoc/search-index.rs stdout ----

error: rustdoc failed!
status: exit code: 101
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/search-index/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/search-index" "/checkout/src/test/rustdoc/search-index.rs"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'Deref section without derived id', src/librustdoc/html/render/mod.rs:4419:18

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.
note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md
note: rustc 1.51.0-nightly (1d7fcae76 2021-01-03) running on x86_64-unknown-linux-gnu

query stack during panic:
end of query stack
---
test result: FAILED. 406 passed; 1 failed; 2 ignored; 0 measured; 0 filtered out; finished in 34.13s



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "rustdoc" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-9/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "9.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver engine executionengine fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86utils xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:19:13

[jryans]

Now that #80661 has merged, there are a few conflicts, so I'll clean those up.