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

Prefer regions with an external_name in approx_universal_upper_bound #78164

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

Aaron1011
Copy link
Member

Fixes #75785

When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static

To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785

This PR approx_universal_upper_bound to prefer regions with an
external_name. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2020
@Aaron1011 Aaron1011 force-pushed the fix/async-region-name branch from 5d745e3 to 02320d4 Compare October 20, 2020 23:24
@Aaron1011
Copy link
Member Author

@estebank Are there any changes that you'd like me to make?

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-diagnostics Area: Messages for errors, warnings, and lints and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2020
--> $DIR/issue-75785-confusing-named-region.rs:9:5
|
LL | pub async fn async_fn(x: &mut i32) -> (&i32, &i32) {
| - let's call the lifetime of this reference `'1`
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the highlight points at the argument instead of the return type, this fixes #74072.

This means this PR will change the output of the test I added in #76468, so it will need rebasing on top of that.

@Aaron1011
Copy link
Member Author

ping @estebank for review or reassignment

@tmandry
Copy link
Member

tmandry commented Dec 17, 2020

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Dec 17, 2020

📌 Commit 02320d4b4122d1c5f0ccd4ca7e140a79ec5d9695 has been approved by tmandry

@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 Dec 17, 2020
@bors
Copy link
Contributor

bors commented Dec 17, 2020

⌛ Testing commit 02320d4b4122d1c5f0ccd4ca7e140a79ec5d9695 with merge d594f0f4b3745bfd16a2c9a005059bdfc4c19e63...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
diff of stderr:

2   --> $DIR/issue-74072-lifetime-name-annotations.rs:9:5
3    |
4 LL | pub async fn async_fn(x: &mut i32) -> &i32 {
-    |                                       - let's call the lifetime of this reference `'1`
+    |                          - let's call the lifetime of this reference `'1`
6 LL |     let y = &*x;
7    |             --- borrow of `*x` occurs here
8 LL |     *x += 1;

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/async-await/issue-74072-lifetime-name-annotations/issue-74072-lifetime-name-annotations.stderr
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/async-await/issue-74072-lifetime-name-annotations/issue-74072-lifetime-name-annotations.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args async-await/issue-74072-lifetime-name-annotations.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/async-await/issue-74072-lifetime-name-annotations.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/async-await/issue-74072-lifetime-name-annotations" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2018" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/async-await/issue-74072-lifetime-name-annotations/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error[E0506]: cannot assign to `*x` because it is borrowed
   |
   |
LL | pub async fn async_fn(x: &mut i32) -> &i32 {
   |                          - let's call the lifetime of this reference `'1`
LL |     let y = &*x;
   |             --- borrow of `*x` occurs here
LL |     *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
   |     ^^^^^^^ assignment to borrowed `*x` occurs here
LL |     y
   |     - returning this value requires that `*x` is borrowed for `'1`

error[E0506]: cannot assign to `*x` because it is borrowed
   |
   |
LL |         let y = &*x;
   |                 --- borrow of `*x` occurs here
LL |         *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
   |         ^^^^^^^ assignment to borrowed `*x` occurs here
LL |         y
   |         - returning this value requires that `*x` is borrowed for `'1`
LL |     })()
   |     - return type of async closure is &'1 i32

error[E0506]: cannot assign to `*x` because it is borrowed
   |
   |
LL |     (async move || -> &i32 {
   |                       - let's call the lifetime of this reference `'1`
LL |         let y = &*x;
   |                 --- borrow of `*x` occurs here
LL |         *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
   |         ^^^^^^^ assignment to borrowed `*x` occurs here
LL |         y
   |         - returning this value requires that `*x` is borrowed for `'1`

error[E0506]: cannot assign to `*x` because it is borrowed
   |
   |
LL |         let y = &*x;
   |                 --- borrow of `*x` occurs here
LL |         *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
   |         ^^^^^^^ assignment to borrowed `*x` occurs here
LL |         y
   |         - returning this value requires that `*x` is borrowed for `'1`
LL |     }
   |     - return type of async block is &'1 i32
error: aborting due to 4 previous errors

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

---

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/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--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:44

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@bors
Copy link
Contributor

bors commented Dec 17, 2020

💔 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 Dec 17, 2020
Fixes rust-lang#75785

When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static

To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue rust-lang#75785

This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
@Aaron1011 Aaron1011 force-pushed the fix/async-region-name branch from 02320d4 to 419d3ae Compare December 17, 2020 18:24
@Aaron1011
Copy link
Member Author

@bors r=tmandry

@bors
Copy link
Contributor

bors commented Dec 17, 2020

📌 Commit 419d3ae has been approved by tmandry

@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 Dec 17, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#78164 (Prefer regions with an `external_name` in `approx_universal_upper_bound`)
 - rust-lang#80003 (Fix overflow when converting ZST Vec to VecDeque)
 - rust-lang#80023 (Enhance error message when misspelled label to value in break expression)
 - rust-lang#80046 (Add more documentation to `Diagnostic` and `DiagnosticBuilder`)
 - rust-lang#80109 (Remove redundant and unreliable coverage test results)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Dec 18, 2020

⌛ Testing commit 419d3ae with merge f3800db...

@bors bors merged commit 720b694 into rust-lang:master Dec 18, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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.

Lifetime error points to wrong reference in async fn return
9 participants