rustdoc: Remove `ResolvedPath.did` #91195

camelid · 2021-11-24T20:13:25Z

ResolvedPath.did was not actually the same as .path.def_id(). Instead,
.did referred to the DefId of the page to be used as a hyperlink target.
For example, a link to Struct::method() would use Struct's DefId as its
.did field. This behavior is confusing, easy to accidentally misuse, and can
instead be obtained on-demand when computing hyperlink targets. It's also likely
part of the reason kind_side_channel exists. I'm currently working on some
experimental refactorings in collect_intra_doc_links that I believe require --
or at least benefit from -- removing .did.

r? @jyn514

Before, if `register_res` were called on an associated item or enum variant, it would return the parent's `DefId`. Now, it returns the actual `DefId`. This change is a step toward removing `Type::ResolvedPath.did` and potentially removing `kind_side_channel` in rustdoc. It also just simplifies rustdoc's behavior.

jyn514

I'm not really comfortable approving this ... I like the idea in theory, but it seems very strange that none of the tests broke, and I worry something has broken without us realizing.

src/librustdoc/clean/utils.rs

src/librustdoc/clean/types.rs

src/librustdoc/clean/utils.rs

camelid · 2021-11-24T21:53:12Z

but it seems very strange that none of the tests broke

I'm not sure if it's very strange; I did update the href code (which IIRC caused lots of tests to fail before I updated it).

jyn514 · 2021-11-24T23:25:54Z

No stdlib changes from #91195 :)

@bors r+

bors · 2021-11-24T23:25:55Z

📌 Commit 4365112 has been approved by jyn514

jyn514 · 2021-11-24T23:26:28Z

Oh, it would be nice to fix #91195 (comment) either here or in a follow up though.

jyn514 · 2021-11-24T23:27:14Z

And I forgot about #91195 (comment) 🤦 r=me with those two done though

@bors r-

Co-authored-by: Joshua Nelson <github@jyn.dev>

camelid · 2021-11-24T23:46:33Z

@bors r=jyn514 rollup=iffy (to make bisecting easier if necessary)

bors · 2021-11-24T23:46:34Z

📌 Commit 617bbb2 has been approved by jyn514

bors · 2021-11-25T12:01:04Z

⌛ Testing commit 617bbb2 with merge 49b989201328d8a1714cffe31906a0a40ad92044...

bors · 2021-11-25T12:33:23Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-11-25T12:34:16Z

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

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

---- [ui] ui/modules/mod-view-items.rs stdout ----

error: test run failed!
status: exit status: 101
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/remote-test-client" "run" "0" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/modules/mod-view-items/a"
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=arm-unknown-linux-gnueabihf
------------------------------------------
uploaded "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/modules/mod-view-items/a", waiting for result


------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'client.read_exact(&mut header) failed with Connection reset by peer (os error 104)', src/tools/remote-test-client/src/main.rs:310:9

------------------------------------------


---
test result: FAILED. 12225 passed; 1 failed; 183 ignored; 0 measured; 0 filtered out; finished in 663.02s



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/arm-unknown-linux-gnueabihf/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-arm-unknown-linux-gnueabihf" "--suite" "ui" "--mode" "ui" "--target" "arm-unknown-linux-gnueabihf" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--linker" "arm-linux-gnueabihf-gcc" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/arm-unknown-linux-gnueabihf/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--llvm-version" "13.0.0-rust-1.58.0-nightly" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets 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 cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker dwp engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink libdriver lineeditor linker lto m68k m68kasmparser m68kcodegen m68kdesc m68kdisassembler m68kinfo 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 orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo 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 webassemblyutils windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--remote-test-client" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/remote-test-client" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"


Build completed unsuccessfully in 0:28:43

jyn514 · 2021-11-25T12:57:35Z

@bors retry

bors · 2021-11-25T13:11:39Z

⌛ Testing commit 617bbb2 with merge 862962b...

bors · 2021-11-25T16:11:58Z

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 862962b to master...

rust-timer · 2021-11-25T17:41:56Z

Finished benchmarking commit (862962b): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

camelid · 2021-11-25T20:06:52Z

Instructions improvements up to -0.1% and max-rss improvements up to -1% on doc builds :D

camelid added 3 commits November 24, 2021 12:00

Use path.def_id() in Type::inner_def_id()

db4a06e

Remove ResolvedPath.did

4365112

camelid added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 24, 2021

rust-highfive assigned jyn514 Nov 24, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2021

camelid mentioned this pull request Nov 24, 2021

rustdoc: Rename Type::ResolvedPath to Type::Path and don't re-export it #91197

Merged

jyn514 reviewed Nov 24, 2021

View reviewed changes

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

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

src/librustdoc/clean/utils.rs Show resolved Hide resolved

src/librustdoc/clean/utils.rs Show resolved Hide resolved

jyn514 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 Nov 24, 2021

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 24, 2021

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 24, 2021

Update comment

617bbb2

Co-authored-by: Joshua Nelson <github@jyn.dev>

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 24, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 25, 2021

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 25, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2021

bors merged commit 862962b into rust-lang:master Nov 25, 2021

rustbot added this to the 1.58.0 milestone Nov 25, 2021

camelid deleted the path-did branch November 25, 2021 20:05

camelid added I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-compiletime Issue: Problems and improvements with respect to compile times. labels Nov 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Remove `ResolvedPath.did` #91195

rustdoc: Remove `ResolvedPath.did` #91195

camelid commented Nov 24, 2021

jyn514 left a comment

camelid commented Nov 24, 2021

jyn514 commented Nov 24, 2021

bors commented Nov 24, 2021

jyn514 commented Nov 24, 2021

jyn514 commented Nov 24, 2021

camelid commented Nov 24, 2021

bors commented Nov 24, 2021

bors commented Nov 25, 2021

bors commented Nov 25, 2021

rust-log-analyzer commented Nov 25, 2021

jyn514 commented Nov 25, 2021

bors commented Nov 25, 2021

bors commented Nov 25, 2021

rust-timer commented Nov 25, 2021

camelid commented Nov 25, 2021

rustdoc: Remove ResolvedPath.did #91195

rustdoc: Remove ResolvedPath.did #91195

Conversation

camelid commented Nov 24, 2021

jyn514 left a comment

Choose a reason for hiding this comment

camelid commented Nov 24, 2021

jyn514 commented Nov 24, 2021

bors commented Nov 24, 2021

jyn514 commented Nov 24, 2021

jyn514 commented Nov 24, 2021

camelid commented Nov 24, 2021

bors commented Nov 24, 2021

bors commented Nov 25, 2021

bors commented Nov 25, 2021

rust-log-analyzer commented Nov 25, 2021

jyn514 commented Nov 25, 2021

bors commented Nov 25, 2021

bors commented Nov 25, 2021

rust-timer commented Nov 25, 2021

camelid commented Nov 25, 2021

rustdoc: Remove `ResolvedPath.did` #91195

rustdoc: Remove `ResolvedPath.did` #91195