Remove RefCell from `JsonRenderer.index` #82354

camelid · 2021-02-21T01:22:02Z

No description provided.

rust-highfive · 2021-02-21T01:22:04Z

(rust-highfive has picked a reviewer for you, use r? to override)

camelid · 2021-02-21T01:23:37Z

src/librustdoc/json/mod.rs

-        let mut index = (*self.index).clone().into_inner();
+        let mut index = self.index.clone();


I'm not fully sure the behavior is the same here. It seems like before it was moving out of an Rc, which I didn't think was possible, so I suspect I'm misunderstanding what this is doing.

I wonder if we could avoid the clone altogether and just push directly into self.index, but I don't know this code, so that may break stuff.

Okay, looks like I did change the behavior, because every test is panicking 😆

It doesn't move outside the Rc, it just forces to dereference it and make it so that clone is called on the RefCell instead of the Rc. This code should have kept its meaning.

What changed however is that now cloneing a JsonRenderer will create a totally separate index. In particular the insert in the item method will have effect only on self's index while before it affected any JsonRenderer with the same origin.

The only way for this to work is to remove all calls to clone(). If you get that to work I would expect it to have the same behavior as before.

Thanks for the explanation! Probably this should wait then on #82356.

rust-log-analyzer · 2021-02-21T01:55:41Z

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)

.................................................................................................... 9200/11472
.................................................................................................... 9300/11472
.................................................................................................... 9400/11472
.............................i......i............................................................... 9500/11472
....................................................................iiiiiii..iiiiii.i............... 9600/11472
.................................................................................................... 9800/11472
.................................................................................................... 9900/11472
.................................................................................................... 10000/11472
.................................................................................................... 10100/11472
---
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 29 tests
iiiiiiiiiiiiiiiiiiiiiiiiiiiii

 finished in 0.075 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.33s

 finished in 2.413 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
---- [rustdoc-json] rustdoc-json/traits/has_body.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/traits/has_body/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/has_body" "/checkout/src/test/rustdoc-json/traits/has_body.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/methods/header.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/methods/header/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/methods/header" "/checkout/src/test/rustdoc-json/methods/header.rs" "--edition=2018" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/structs/plain_empty.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/structs/plain_empty/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/structs/plain_empty" "/checkout/src/test/rustdoc-json/structs/plain_empty.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/fns/header.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/fns/header/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/fns/header" "/checkout/src/test/rustdoc-json/fns/header.rs" "--edition=2018" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/method_abi.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/method_abi/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/method_abi" "/checkout/src/test/rustdoc-json/method_abi.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/structs/with_primitives.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/structs/with_primitives/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/structs/with_primitives" "/checkout/src/test/rustdoc-json/structs/with_primitives.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/structs/with_generics.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/structs/with_generics/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/structs/with_generics" "/checkout/src/test/rustdoc-json/structs/with_generics.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/fn_pointer/header.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/fn_pointer/header/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/fn_pointer/header" "/checkout/src/test/rustdoc-json/fn_pointer/header.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/structs/unit.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/structs/unit/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/structs/unit" "/checkout/src/test/rustdoc-json/structs/unit.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/unions/union.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/unions/union/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/unions/union" "/checkout/src/test/rustdoc-json/unions/union.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/structs/tuple.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/structs/tuple/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/structs/tuple" "/checkout/src/test/rustdoc-json/structs/tuple.rs" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
---- [rustdoc-json] rustdoc-json/nested.rs stdout ----

error: rustdoc failed!
status: exit code: 1
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-json/nested/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json/nested" "/checkout/src/test/rustdoc-json/nested.rs" "--edition=2018" "--output-format" "json"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustdoc/json/mod.rs:248:47

error: internal compiler error: unexpected panic

error: Unrecognized option: 'output-format'
---
test result: FAILED. 0 passed; 12 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.06s



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" "--jsondocck-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/jsondocck" "--src-base" "/checkout/src/test/rustdoc-json" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-json" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "rustdoc-json" "--mode" "rustdoc-json" "--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/python3" "--lldb-python" "/usr/bin/python3" "--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:30:59

camelid · 2021-03-12T04:11:22Z

#82356 has been merged, so this is no longer blocked! 🎉

camelid · 2021-03-17T23:06:00Z

Based on re-reading #82354 (comment) and thinking about this a bit more, I don't think this change is possible without significantly refactoring FormatRenderer (which would allow removing shared state from Context as well as JsonRenderer). So, I think this PR should probably be closed.

camelid · 2021-03-17T23:26:29Z

I think I'll leave this open for a bit in case anyone has ideas.

jyn514 · 2021-03-18T03:59:14Z

I don't think there's much point in leaving this open if it doesn't work. Thanks for trying this, though!

Remove RefCell from JsonRenderer.index

e9e5863

camelid added A-rustdoc-json Area: Rustdoc JSON backend 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 Feb 21, 2021

rust-highfive assigned jyn514 Feb 21, 2021

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

camelid commented Feb 21, 2021

View reviewed changes

camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 12, 2021

jyn514 closed this Mar 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove RefCell from `JsonRenderer.index` #82354

Remove RefCell from `JsonRenderer.index` #82354

camelid commented Feb 21, 2021

rust-highfive commented Feb 21, 2021

camelid Feb 21, 2021

camelid Feb 21, 2021

SkiFire13 Feb 21, 2021

jyn514 Feb 21, 2021

camelid Feb 21, 2021

rust-log-analyzer commented Feb 21, 2021

camelid commented Mar 12, 2021

camelid commented Mar 17, 2021

camelid commented Mar 17, 2021

jyn514 commented Mar 18, 2021

		let mut index = (*self.index).clone().into_inner();
		let mut index = self.index.clone();

Remove RefCell from JsonRenderer.index #82354

Remove RefCell from JsonRenderer.index #82354

Conversation

camelid commented Feb 21, 2021

rust-highfive commented Feb 21, 2021

camelid Feb 21, 2021

Choose a reason for hiding this comment

camelid Feb 21, 2021

Choose a reason for hiding this comment

SkiFire13 Feb 21, 2021

Choose a reason for hiding this comment

jyn514 Feb 21, 2021

Choose a reason for hiding this comment

camelid Feb 21, 2021

Choose a reason for hiding this comment

rust-log-analyzer commented Feb 21, 2021

camelid commented Mar 12, 2021

camelid commented Mar 17, 2021

camelid commented Mar 17, 2021

jyn514 commented Mar 18, 2021

Remove RefCell from `JsonRenderer.index` #82354

Remove RefCell from `JsonRenderer.index` #82354