Use ufcs in derive(Debug) #81294

pnkfelix · 2021-01-23T05:55:48Z

(Arguably this is the fix for it.)

rust-highfive · 2021-01-23T05:55:51Z

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

pnkfelix · 2021-01-23T05:57:48Z

There is one thing I was curious about: I used cx.std_path because I thought that was necessary to get the right behavior with respect to whether core or std is emitted, but I do see use of some sort of path_std! macro, which seems much nicer.

~~Should I be using path_std! here as well, then? I plan to give it a whirl at some point. But I figured I could put this up for initial review feedback in the meantime.~~

Answer: No, path_std! is not applicable here. It yields ty::Path values, and I need a Vec<Ident> to pass to expr_call_global...

rust-log-analyzer · 2021-01-23T06:25:10Z

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)


---- [ui] ui/panic-handler/weak-lang-item.rs stdout ----
diff of stderr:

10 LL | extern crate core as other_core;
12 
12 
- error: language item required, but not found: `eh_personality`
- 
15 error: `#[panic_handler]` function required, but not found
+ 
+ error: language item required, but not found: `eh_personality`
17 error: aborting due to 3 previous errors
18 



The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-handler/weak-lang-item/weak-lang-item.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args panic-handler/weak-lang-item.rs`
error: 1 errors occurred comparing output.
status: exit code: 1
status: exit code: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/panic-handler/weak-lang-item.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-handler/weak-lang-item" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-handler/weak-lang-item/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error[E0259]: the name `core` is defined multiple times
   |
LL | extern crate core;
LL | extern crate core;
   | ^^^^^^^^^^^^^^^^^^ `core` reimported here
   |
   = note: `core` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
   |
LL | extern crate core as other_core;


error: `#[panic_handler]` function required, but not found

error: language item required, but not found: `eh_personality`
error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0259`.

---

Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu


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" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--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:16:22

pnkfelix · 2021-01-23T06:26:00Z

(I'm seeing that failure locally too, but I'm at a loss for how what I did could have caused it...)

pnkfelix · 2021-01-23T06:27:13Z

(marking as S-waiting-on-author since it cannot land as is, but I still would welcome any hints as to what I might have done wrong.)

oli-obk · 2021-01-23T08:36:54Z

I believe I have seen the opposite change recently. This may be a spuriously fluctuating diagnostic (or semi-spurious, depending on random but fixed things in the compiler, since I have not seen it happen randomly on CI).

oli-obk

r=me with the "spurious" test blessed. I'll open an issue about it

oli-obk · 2021-01-23T09:30:42Z

compiler/rustc_builtin_macros/src/deriving/debug.rs

            stmts.push(cx.stmt_let(span, true, builder, expr));

            for field in fields {
                // Use double indirection to make sure this works for unsized types
                let field = cx.expr_addr_of(field.span, field.self_.clone());
                let field = cx.expr_addr_of(field.span, field);

-                let expr = cx.expr_method_call(


Maybe we should just remove the expr_method_call function entirely (or perma-deprecate it so ppl searching for it get an explanation of why they should use ufcs)

Yeah I considered this as well. Probably a good idea, especially if this is the last use of it. I'll try to look into that.

pnkfelix · 2021-01-25T16:52:58Z

@bors r=oli-obk

bors · 2021-01-25T16:53:00Z

📌 Commit 24149d7 has been approved by oli-obk

Mark-Simulacrum · 2021-01-25T19:43:33Z

@bors rollup=never potentially perf sensitive

bors · 2021-01-26T08:59:15Z

⌛ Testing commit 24149d7 with merge c27291b43e30556440e671568db27c2795cc8dae...

JohnTitor · 2021-01-26T09:07:16Z

@bors retry yielding

rust-log-analyzer · 2021-01-26T09:43:15Z

A job failed! Check out the build log: (web) (plain)

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

bors · 2021-01-26T10:43:41Z

⌛ Testing commit 24149d7 with merge 087a6a1947865c637bec869ed2217b4f5e5d6829...

bors · 2021-01-26T11:15:36Z

💔 Test failed - checks-actions

jonas-schievink · 2021-01-30T16:38:15Z

@bors r-

…for discussion.

… method collisions I could imagine. The different impls are all guarded by cfg-flags, and the revisions could be used to cover the full power-set of combinations. (I only included 20 of the possible 32 cases here; the null-set is not interesting, and the remaining 11 all yielded ambiguous method resolution errors which did not mix well with this testing strategy; I'm not trying to check UI for the resolution diagnostics; I'm trying to create checkpoint of current resolution semantics when compilation succeeds.)

pnkfelix · 2021-02-01T22:10:27Z

oh you've got to be kidding me, my local systems failure to test coverage is biting me again, just like in PR #81257?

Okay, I guess its time to invest a little more effort in figuring out what is supposed to be running and why my system isn't running it...

oli-obk · 2021-02-03T11:48:34Z

@bors r+

bors · 2021-02-03T11:48:35Z

📌 Commit 1783c47 has been approved by oli-obk

bors · 2021-02-03T15:12:25Z

⌛ Testing commit 1783c47 with merge 186f7ae...

bors · 2021-02-03T18:01:28Z

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

tmiasko · 2021-02-04T18:44:44Z

Perf results: https://perf.rust-lang.org/compare.html?start=6ad11e2e25919b75ebbc36d7910f2a1126a7e873&end=186f7ae5b04d31d8ccd1746ac63cdf1ab4bc2354&stat=instructions:u

Note: deeply-nested-async, match-stress-enum, regression-31157, (and some others) benchmarks doesn't have any derives.

EDIT: The MIR is larger to due an extra reborrow for each method call. Opened #81760 to determine if this impacts perf.

…=varkor Remove usages of `expr_method_call` in derive(Ord,PartialOrd,RustcEncode,RustcDecode) Preparing for deprecation of `expr_method_call` (rust-lang#81295), by removing the remaining usages not covered by (rust-lang#81294). I am not sure about the changes to `derive(RustcEncode,RustcDecode)`

tmiasko · 2021-02-10T12:38:28Z

The experiment from #81760, while produces smaller MIR, didn't help with regressions. The comparisons before & #81760 (insert extra caveats about the non-adjacent comparison ...) https://perf.rust-lang.org/compare.html?start=6ad11e2e25919b75ebbc36d7910f2a1126a7e873&end=d589fc7d554c7ecdab26eb7ae07fd6dc7e8280f7&stat=instructions%3Au

There are additional queries for type_op_ascribe_user_type and regression in typeck. The new MIR does indeed contain additional user type annotations. Looking at how they are used, they will involve extra work equating & relating the types, so this might be an aspect that contributes to the regressions.

oli-obk · 2021-02-11T12:25:17Z

Oh... so UFCs always were less efficient than method calls in terms of compile-time or at least query invocation count. I wonder if we can adjust the relevant call sites to not use type_op_ascribe_user_type where it's "obviously" not necessary. Potentially by wrapping that query in a function that does some early aborts.

Not sure what changed in typeck, will need to dig into the code in order to figure that out

rust-highfive assigned oli-obk Jan 23, 2021

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

pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 23, 2021

pnkfelix force-pushed the issue-81211-use-ufcs-in-derive-debug branch from 3283d15 to aa8fdad Compare January 23, 2021 06:05

pnkfelix 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 Jan 23, 2021

oli-obk approved these changes Jan 23, 2021

View reviewed changes

This was referenced Jan 23, 2021

Deprecate the expr_method_call function entirely in favour of ufc #81295

Closed

Semi-spurious ui test fluctuation #81296

Closed

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

This was referenced Jan 25, 2021

Rollup of 11 pull requests #81390

Closed

Rollup of 11 pull requests #81392

Closed

pnkfelix mentioned this pull request Jan 25, 2021

Make weak item traversal deterministic #81393

Merged

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2021

pnkfelix added 4 commits February 1, 2021 17:08

Use UFCS instead of method calls in derive(Debug). See issue 81211 …

2307d08

…for discussion.

Regression tests for issue 81211.

ff4665f

placate tidy.

a7745d9

bless the coverage-spanview output. Cc rust-lang#81688.

1783c47

pnkfelix force-pushed the issue-81211-use-ufcs-in-derive-debug branch from 78409b0 to 1783c47 Compare February 3, 2021 03:29

pnkfelix mentioned this pull request Feb 3, 2021

Use # min-llvm-version: 11.0 to force a minimum LLVM version #81688

Merged

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 Feb 3, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 3, 2021

bors merged commit 186f7ae into rust-lang:master Feb 3, 2021

rustbot added this to the 1.51.0 milestone Feb 3, 2021

This was referenced Nov 23, 2022

Method resolution error #81211

Closed

Need to document how method resolution ambiguties are/aren't resolved #104783

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ufcs in derive(Debug) #81294

Use ufcs in derive(Debug) #81294

pnkfelix commented Jan 23, 2021

rust-highfive commented Jan 23, 2021

pnkfelix commented Jan 23, 2021 •

edited

Loading

rust-log-analyzer commented Jan 23, 2021

pnkfelix commented Jan 23, 2021

pnkfelix commented Jan 23, 2021

oli-obk commented Jan 23, 2021

oli-obk left a comment

oli-obk Jan 23, 2021

pnkfelix Jan 25, 2021

pnkfelix commented Jan 25, 2021

bors commented Jan 25, 2021

Mark-Simulacrum commented Jan 25, 2021

bors commented Jan 26, 2021

JohnTitor commented Jan 26, 2021

rust-log-analyzer commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

jonas-schievink commented Jan 30, 2021

pnkfelix commented Feb 1, 2021

oli-obk commented Feb 3, 2021

bors commented Feb 3, 2021

bors commented Feb 3, 2021

bors commented Feb 3, 2021

tmiasko commented Feb 4, 2021 •

edited

Loading

tmiasko commented Feb 10, 2021 •

edited

Loading

oli-obk commented Feb 11, 2021

Use ufcs in derive(Debug) #81294

Use ufcs in derive(Debug) #81294

Conversation

pnkfelix commented Jan 23, 2021

rust-highfive commented Jan 23, 2021

pnkfelix commented Jan 23, 2021 • edited Loading

rust-log-analyzer commented Jan 23, 2021

pnkfelix commented Jan 23, 2021

pnkfelix commented Jan 23, 2021

oli-obk commented Jan 23, 2021

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk Jan 23, 2021

Choose a reason for hiding this comment

pnkfelix Jan 25, 2021

Choose a reason for hiding this comment

pnkfelix commented Jan 25, 2021

bors commented Jan 25, 2021

Mark-Simulacrum commented Jan 25, 2021

bors commented Jan 26, 2021

JohnTitor commented Jan 26, 2021

rust-log-analyzer commented Jan 26, 2021

bors commented Jan 26, 2021

bors commented Jan 26, 2021

jonas-schievink commented Jan 30, 2021

pnkfelix commented Feb 1, 2021

oli-obk commented Feb 3, 2021

bors commented Feb 3, 2021

bors commented Feb 3, 2021

bors commented Feb 3, 2021

tmiasko commented Feb 4, 2021 • edited Loading

tmiasko commented Feb 10, 2021 • edited Loading

oli-obk commented Feb 11, 2021

pnkfelix commented Jan 23, 2021 •

edited

Loading

tmiasko commented Feb 4, 2021 •

edited

Loading

tmiasko commented Feb 10, 2021 •

edited

Loading