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

Suggest as_mut when Pin<T> is used after move #93181

Closed
wants to merge 2 commits into from

Conversation

ibraheemdev
Copy link
Member

@ibraheemdev ibraheemdev commented Jan 22, 2022

let foo = Pin::new(&mut foo);
foo.foo();
foo.foo();
help: consider calling `.as_mut()` to borrow the type's contents
   |
2  |     foo.as_mut().foo();
   |         +++++++++

Resolves #65409

@rust-highfive
Copy link
Collaborator

r? @lcnr

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

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 22, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2022
/// Used for better diagnostics.
is_option_or_result: bool,
self_name: Option<Symbol>,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self_name: Option<Symbol>,
self_diagnostic_name: Option<Symbol>,

@lcnr
Copy link
Contributor

lcnr commented Jan 24, 2022

r=me after @nagisa s comment

@lcnr lcnr 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 25, 2022
Copy link
Contributor

@danielhenrymantilla danielhenrymantilla left a comment

Choose a reason for hiding this comment

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

Oh yes, very nice!

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs Outdated Show resolved Hide resolved
err.span_suggestion_verbose(
fn_call_span.shrink_to_lo(),
&format!(
"consider calling `.{}()` to borrow the type's contents",
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra nit, but I personally would love that level of precision in the diagnostics: in the case where P = &mut _, that is, when we have to deal with Pin<&mut …>, we already have a borrow; and what we need is a re-borrow.

  • This may look silly, but I think one of the main areas of confusion with Pin for beginners is this need for .as_mut(), precisely because it's a type which, contrary to &mut, features no implicit reborrowing.

    So even an extra note for that very case, stating this very fact, could be quite educational:

    note: this function takes ownership of the receiver `self`, which moves `foo`
      --> $DIR/pin-content-move.rs:6:12
       |
    LL |     fn foo(self: Pin<&mut Self>) {}
       |            ^^^^
    help: consider calling `.as_mut()` to reborrow the type's contents
       |
    LL |     foo.as_mut().foo();
       |         +++++++++
    note: this is needed because `Pin<&mut _>`, contrary to `&mut _`, does not feature implicit reborrowing.
       |
    LL |     foo.as_mut().foo();
       |         +++++++++
  • While that instance seems like a nit, in cases such as Pin<Rc<…>> etc. for shared-pinned stuff, the .as_mut() suggestion is also likely to be incorrect.

    So, if it weren't too annoying to implement, I'd do a heuristic whereby Rc, Arc, and &mut _ would be special-cased, and would ask for, respectively: .as_ref() to borrow, .as_ref() to borrow, .as_mut() to reborrow, and otherwise defaulting to what we have here: .as_mut() to borrow.

Co-authored-by: Daniel Henry-Mantilla <daniel.henry.mantilla@gmail.com>
@ibraheemdev
Copy link
Member Author

It looks like there's a false positive when the receiver actually takes self by value. We may need to do some type-checking to avoid this.

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
---- [ui] ui/moves/move-fn-self-receiver.rs stdout ----
diff of stderr:

70    |
71 LL |     fn use_pin_box_self(self: Pin<Box<Self>>) {}
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
72    |                         ^^^^
+ help: consider calling `.as_mut()` to borrow the type's contents
+    |
+ LL |     pin_box_foo.as_mut().use_pin_box_self();
73 
73 
74 error[E0505]: cannot move out of `mut_foo` because it is borrowed


The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/moves/move-fn-self-receiver/move-fn-self-receiver.stderr
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/moves/move-fn-self-receiver/move-fn-self-receiver.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args moves/move-fn-self-receiver.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/moves/move-fn-self-receiver.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/moves/move-fn-self-receiver" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/moves/move-fn-self-receiver/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error[E0382]: use of moved value: `val.0`
   |
   |
LL |     val.0.into_iter().next();
   |           ----------- `val.0` moved due to this method call
LL |     val.0; //~ ERROR use of moved
   |     ^^^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `val.0`
   |
LL |     fn into_iter(self) -> Self::IntoIter;
   |                  ^^^^
   |                  ^^^^
   = note: move occurs because `val.0` has type `Vec<bool>`, which does not implement the `Copy` trait
error[E0382]: use of moved value: `foo`
  --> /checkout/src/test/ui/moves/move-fn-self-receiver.rs:34:5
   |
LL |     let foo = Foo;
LL |     let foo = Foo;
   |         --- move occurs because `foo` has type `Foo`, which does not implement the `Copy` trait
LL |     foo.use_self();
   |         ---------- `foo` moved due to this method call
LL |     foo; //~ ERROR use of moved
   |     ^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `foo`
   |
LL |     fn use_self(self) {}
   |                 ^^^^


error[E0382]: use of moved value: `second_foo`
   |
LL |     let second_foo = Foo;
LL |     let second_foo = Foo;
   |         ---------- move occurs because `second_foo` has type `Foo`, which does not implement the `Copy` trait
LL |     second_foo.use_self();
   |                ---------- `second_foo` moved due to this method call
LL |     second_foo; //~ ERROR use of moved
   |     ^^^^^^^^^^ value used here after move

error[E0382]: use of moved value: `boxed_foo`
   |
   |
LL |     let boxed_foo = Box::new(Foo);
   |         --------- move occurs because `boxed_foo` has type `Box<Foo>`, which does not implement the `Copy` trait
LL |     boxed_foo.use_box_self();
   |               -------------- `boxed_foo` moved due to this method call
LL |     boxed_foo; //~ ERROR use of moved
   |     ^^^^^^^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `boxed_foo`
   |
   |
LL |     fn use_box_self(self: Box<Self>) {}


error[E0382]: use of moved value: `pin_box_foo`
   |
   |
LL |     let pin_box_foo = Box::pin(Foo);
   |         ----------- move occurs because `pin_box_foo` has type `Pin<Box<Foo>>`, which does not implement the `Copy` trait
LL |     pin_box_foo.use_pin_box_self();
   |                 ------------------ `pin_box_foo` moved due to this method call
LL |     pin_box_foo; //~ ERROR use of moved
   |     ^^^^^^^^^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `pin_box_foo`
   |
   |
LL |     fn use_pin_box_self(self: Pin<Box<Self>>) {}
   |                         ^^^^
help: consider calling `.as_mut()` to borrow the type's contents
   |
LL |     pin_box_foo.as_mut().use_pin_box_self();


error[E0505]: cannot move out of `mut_foo` because it is borrowed
   |
   |
LL |     let ret = mut_foo.use_mut_self();
   |               ---------------------- borrow of `mut_foo` occurs here
LL |     mut_foo; //~ ERROR cannot move out
   |     ^^^^^^^ move out of `mut_foo` occurs here
LL |     ret;


error[E0382]: use of moved value: `rc_foo`
   |
   |
LL |     let rc_foo = Rc::new(Foo);
   |         ------ move occurs because `rc_foo` has type `Rc<Foo>`, which does not implement the `Copy` trait
LL |     rc_foo.use_rc_self();
   |            ------------- `rc_foo` moved due to this method call
LL |     rc_foo; //~ ERROR use of moved
   |     ^^^^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `rc_foo`
   |
   |
LL |     fn use_rc_self(self: Rc<Self>) {}


error[E0382]: use of moved value: `foo_add`
   |
LL |     let foo_add = Foo;
LL |     let foo_add = Foo;
   |         ------- move occurs because `foo_add` has type `Foo`, which does not implement the `Copy` trait
LL |     foo_add + Foo;
   |     ------------- `foo_add` moved due to usage in operator
LL |     foo_add; //~ ERROR use of moved
   |     ^^^^^^^ value used here after move
note: calling this operator moves the left-hand side
  --> /checkout/library/core/src/ops/arith.rs:114:12
   |
   |
LL |     fn add(self, rhs: Rhs) -> Self::Output;

error[E0382]: use of moved value: `implicit_into_iter`
  --> /checkout/src/test/ui/moves/move-fn-self-receiver.rs:63:5
   |
   |
LL |     let implicit_into_iter = vec![true];
   |         ------------------ move occurs because `implicit_into_iter` has type `Vec<bool>`, which does not implement the `Copy` trait
LL |     for _val in implicit_into_iter {}
   |                 |
   |                 |
   |                 `implicit_into_iter` moved due to this implicit call to `.into_iter()`
   |                 help: consider borrowing to avoid moving into the for loop: `&implicit_into_iter`
LL |     implicit_into_iter; //~ ERROR use of moved
   |     ^^^^^^^^^^^^^^^^^^ value used here after move
error[E0382]: use of moved value: `explicit_into_iter`
  --> /checkout/src/test/ui/moves/move-fn-self-receiver.rs:67:5
   |
   |
LL |     let explicit_into_iter = vec![true];
   |         ------------------ move occurs because `explicit_into_iter` has type `Vec<bool>`, which does not implement the `Copy` trait
LL |     for _val in explicit_into_iter.into_iter() {}
   |                                    ----------- `explicit_into_iter` moved due to this method call
LL |     explicit_into_iter; //~ ERROR use of moved
   |     ^^^^^^^^^^^^^^^^^^ value used here after move
error[E0382]: use of moved value: `container`
  --> /checkout/src/test/ui/moves/move-fn-self-receiver.rs:71:5
   |
   |
LL |     let container = Container(vec![]);
   |         --------- move occurs because `container` has type `Container`, which does not implement the `Copy` trait
LL |     for _val in container.custom_into_iter() {}
   |                           ------------------ `container` moved due to this method call
LL |     container; //~ ERROR use of moved
   |     ^^^^^^^^^ value used here after move
   |
note: this function takes ownership of the receiver `self`, which moves `container`
   |
   |
LL |     fn custom_into_iter(self) -> impl Iterator<Item = bool> {

error[E0382]: use of moved value: `foo2`
  --> /checkout/src/test/ui/moves/move-fn-self-receiver.rs:75:9
   |
   |
LL |     let foo2 = Foo;
   |         ---- move occurs because `foo2` has type `Foo`, which does not implement the `Copy` trait
LL |     loop {
LL |         foo2.use_self(); //~ ERROR use of moved
   |         ^^^^ ---------- `foo2` moved due to this method call, in previous iteration of loop
error: aborting due to 12 previous errors

Some errors have detailed explanations: E0382, E0505.
For more information about an error, try `rustc --explain E0382`.
---
test result: FAILED. 12446 passed; 1 failed; 121 ignored; 0 measured; 0 filtered out; finished in 117.16s



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-12/bin/FileCheck" "--nodejs" "/usr/bin/node" "--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/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" "12.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 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 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 orcshared orctargetprocess passes perfjitevents 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 xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"


Build completed unsuccessfully in 0:12:10

@bors
Copy link
Contributor

bors commented Feb 13, 2022

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

@JohnCSimon
Copy link
Member

@ibraheemdev
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this. Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Mar 20, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Mar 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest as_mut to reborrow Pin when calling method
9 participants