Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves migrations lint for RFC2229 #86965

Merged
merged 9 commits into from
Jul 11, 2021

Conversation

roxelo
Copy link
Member

@roxelo roxelo commented Jul 8, 2021

This PR improves the current disjoint capture migration lint by providing more information on why drop order or auto trait implementation for a closure is impacted by the use of the new feature.

The drop order migration lint will now look something like this:

error: changes to closure capture in Rust 2021 will affect drop order
  --> $DIR/significant_drop.rs:163:21
   |
LL |             let c = || {
   |                     ^^
...
LL |                 tuple.0;
   |                 ------- in Rust 2018, closure captures all of `tuple`, but in Rust 2021, it only captures `tuple.0`
...
LL |         }
   |         - in Rust 2018, `tuple` would be dropped here, but in Rust 2021, only `tuple.0` would be dropped here alongside the closure

The auto trait migration lint will now look something like this:

error: changes to closure capture in Rust 2021 will affect `Send` trait implementation for closure
  --> $DIR/auto_traits.rs:14:19
   |
LL |     thread::spawn(move || unsafe {
   |                   ^^^^^^^^^^^^^^ in Rust 2018, this closure would implement `Send` as `fptr` implements `Send`, but in Rust 2021, this closure would no longer implement `Send` as `fptr.0` does not implement `Send`
...
LL |         *fptr.0 = 20;
   |         ------- in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0`

r? @nikomatsakis

Closes rust-lang/project-rfc-2229#54

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2021
@bors
Copy link
Contributor

bors commented Jul 8, 2021

☔ The latest upstream changes (presumably #86982) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@roxelo roxelo force-pushed the rfc2229-improve-lint branch 2 times, most recently from 9b0e94d to 10088df Compare July 8, 2021 22:04
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, these new errors look great!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 9, 2021

📌 Commit a298535076fda53510bca2b84ac6d20659366660 has been approved by nikomatsakis

@bors 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 Jul 9, 2021
@lqd
Copy link
Member

lqd commented Jul 9, 2021

Great job on these !

@bors r=nikomatsakis,lqd

@bors
Copy link
Contributor

bors commented Jul 9, 2021

📌 Commit 08c6167 has been approved by nikomatsakis,lqd

@bors
Copy link
Contributor

bors commented Jul 10, 2021

⌛ Testing commit 08c6167 with merge 09dff05c3eb334d8175bc0f481f4eef9d1566dd8...

@rust-log-analyzer
Copy link
Collaborator

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

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

---- [ui] ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-stack.rs stdout ----
diff of stderr:

1 error: <unknown>:0:0: in function entry_function i32 (i32, i32, i32, i32, i32): secure entry function requires arguments on stack
3 
+ thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
+   left: `LLVMing`,
+   left: `LLVMing`,
+  right: `Codegenning`', /checkout/compiler/rustc_codegen_ssa/src/back/write.rs:1464:21
+ note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+ error: internal compiler error: unexpected panic
+ 
+ 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.55.0-nightly (09dff05c3 2021-07-10) running on x86_64-unknown-linux-gnu
+ 
+ note: compiler flags: -Z threads=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z emit-future-incompat-report -C codegen-units=1 -C prefer-dynamic -C rpath -C debuginfo=0 --crate-type lib
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
+ query stack during panic:
+ end of query stack
4 error: aborting due to previous error
5 
---
To only update this specific test, also pass `--test-args cmse-nonsecure/cmse-nonsecure-entry/params-on-stack.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-stack.rs" "-Zthreads=1" "--error-format" "json" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-stack" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target" "thumbv8m.main-none-eabi" "--crate-type" "lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cmse-nonsecure/cmse-nonsecure-entry/params-on-stack/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: <unknown>:0:0: in function entry_function i32 (i32, i32, i32, i32, i32): secure entry function requires arguments on stack

thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `LLVMing`,
  left: `LLVMing`,
 right: `Codegenning`', /checkout/compiler/rustc_codegen_ssa/src/back/write.rs:1464:21

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.55.0-nightly (09dff05c3 2021-07-10) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z threads=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z emit-future-incompat-report -C codegen-units=1 -C prefer-dynamic -C rpath -C debuginfo=0 --crate-type lib
query stack during panic:
end of query stack
error: aborting due to previous error

---
test result: FAILED. 12021 passed; 1 failed; 92 ignored; 0 measured; 0 filtered out; finished in 105.49s



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" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--host-rustcflags" "-Crpath -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -Cdebuginfo=0  -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" "--llvm-version" "12.0.1-rust-1.55.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 engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hellonew hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink 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 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 windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"


Build completed unsuccessfully in 0:18:18

@bors
Copy link
Contributor

bors commented Jul 10, 2021

💔 Test failed - checks-actions

@bors 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 Jul 10, 2021
@lqd
Copy link
Member

lqd commented Jul 10, 2021

let's see if this was spurious

@bors retry

@bors 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 Jul 10, 2021
@bors
Copy link
Contributor

bors commented Jul 11, 2021

⌛ Testing commit 08c6167 with merge 9f2e753...

@bors
Copy link
Contributor

bors commented Jul 11, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis,lqd
Pushing 9f2e753 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2021
@bors bors merged commit 9f2e753 into rust-lang:master Jul 11, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 11, 2021
@lqd
Copy link
Member

lqd commented Jul 12, 2021

Since this test passed this time, it's likely there's a bug in codegen. When the main thread starts doing LLVM work -- which IIUC can happen before codegen is completed -- and codegen is then aborted: the message handler expects codegen to be completed, causing the assertion to fail as in the log above.

IIRC @michaelwoerister was involved in this feature of codegen with acrichto: do you want to track this somewhere ?

@michaelwoerister
Copy link
Member

@lqd: I opened #87083 regarding this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve explanations in the migration lint
8 participants