Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability #71726

ldm0 · 2020-05-01T00:33:06Z

Implement dereference suggestion when coercing ty::Ref to ty::RawPtr with arbitrary mutability.
Extract the dereference steps into deref_steps(), which removes all the use and pub noise introduced by last PR Suggest deref when coercing ty::Ref to ty::RawPtr #71540, and makes the code more readable.
Use the remove_prefix() closure which makes the prefix removal more readable.
Introduce Applicability as a return value of check_ref to suggest Applicability::Unspecified suggestion.

Special: I found it is not possible to genereate Applicability::MachineApplicable suggestion for situation like this:

use std::ops::Deref;
use std::ops::DerefMut;
struct Bar(u8);
struct Foo(Bar);
struct Emm(Foo);
impl Deref for Bar{
    type Target = u8;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}
impl Deref for Foo {
    type Target = Bar;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}
impl Deref for Emm {
    type Target = Foo;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}
impl DerefMut for Bar{
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.0
    }
}
impl DerefMut for Foo {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.0
    }
}
impl DerefMut for Emm {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.0
    }
}
fn main() {
    let a = Emm(Foo(Bar(0)));
    let _: *mut u8 = &a; //~ ERROR mismatched types
}

We may suggest &mut ***a here, but the a is not declared as mutable variable. And also when processing HIR, it's not possible to check if a is declared as a mutable variable (currently we do borrow checking with MIR). So we cannot ensure that suggestion when coercing immutable reference to mutable pointer is always machine applicable. Therefore I added a Applicability return value in check_ref(). And move the immutable reference -> mutable pointer situation into a sperate test file without run-rustfix. (It seems that run-rustfix will also adopt Applicability::Unspecified suggestion, which is strange)

rust-highfive · 2020-05-01T00:33:10Z

r? @cramertj

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

ldm0 · 2020-05-01T00:34:31Z

r? @oli-obk
Since #71540

src/librustc_typeck/check/coercion.rs

oli-obk · 2020-05-01T09:30:00Z

src/librustc_typeck/check/demand.rs

@@ -470,7 +474,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                        let sugg_expr = if needs_parens { format!("({})", src) } else { src };

                        if let Some(sugg) = self.can_use_as_ref(expr) {
-                            return Some(sugg);
+                            return Some((


I think we're reaching the point where this function should probably return a struct with named fields ;)

But I think for consistency that would requires a lot of irrelevant changes. If that's really need would changing them in a seperate PR be better? :-P

Oh yea, this was mainly a drive by comment, feel free to address it or not or in a different PR

src/librustc_typeck/check/demand.rs

src/test/ui/issues/issue-71676-2.rs

oli-obk · 2020-05-01T15:58:51Z

@bors r+

bors · 2020-05-01T15:58:53Z

📌 Commit a74a048cb7e61f7038f2a48c39cc3d8af37fc354 has been approved by oli-obk

@ghost

Rollup of 5 pull requests Successful merges: - rust-lang#71018 (handle ConstValue::ByRef in relate) - rust-lang#71758 (Remove leftover chalk types) - rust-lang#71760 (Document unsafety for `*const T` and `*mut T`) - rust-lang#71761 (doc: reference does not exist, probably a typo) - rust-lang#71762 (doc: this resulted in a link pointing to a non-existent target) Failed merges: - rust-lang#71726 (Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability) r? @ghost

bors · 2020-05-02T03:54:17Z

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

mutability

as `Applicability::Unspecified`

ldm0 · 2020-05-02T05:53:21Z

Rebase done.

oli-obk · 2020-05-02T06:18:45Z

@bors r+

bors · 2020-05-02T06:18:47Z

📌 Commit 9a212c1 has been approved by oli-obk

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability Fixes rust-lang#71676 1. Implement dereference suggestion when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability. 2. Extract the dereference steps into `deref_steps()`, which removes all the `use` and `pub` noise introduced by last PR rust-lang#71540, and makes the code more readable. 3. Use the `remove_prefix()` closure which makes the prefix removal more readable. 4. Introduce `Applicability` as a return value of `check_ref` to suggest `Applicability::Unspecified` suggestion. **Special**: I found it is not possible to genereate `Applicability::MachineApplicable` suggestion for situation like this: ```rust use std::ops::Deref; use std::ops::DerefMut; struct Bar(u8); struct Foo(Bar); struct Emm(Foo); impl Deref for Bar{ type Target = u8; fn deref(&self) -> &Self::Target { &self.0 } } impl Deref for Foo { type Target = Bar; fn deref(&self) -> &Self::Target { &self.0 } } impl Deref for Emm { type Target = Foo; fn deref(&self) -> &Self::Target { &self.0 } } impl DerefMut for Bar{ fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl DerefMut for Foo { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl DerefMut for Emm { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } fn main() { let a = Emm(Foo(Bar(0))); let _: *mut u8 = &a; //~ ERROR mismatched types } ``` We may suggest `&mut ***a` here, but the `a` is not declared as mutable variable. And also when processing HIR, it's not possible to check if `a` is declared as a mutable variable (currently we do borrow checking with MIR). So we cannot ensure that suggestion when coercing immutable reference to mutable pointer is always machine applicable. Therefore I added a `Applicability` return value in `check_ref()`. And move the `immutable reference -> mutable pointer` situation into a sperate test file without `run-rustfix`. (It seems that `run-rustfix` will also adopt `Applicability::Unspecified` suggestion, which is strange)

@ghost

Rollup of 4 pull requests Successful merges: - rust-lang#71398 (Add `RefCell::take`) - rust-lang#71663 (Fix exceeding bitshifts not emitting for assoc. consts (properly this time, I swear!)) - rust-lang#71726 (Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability) - rust-lang#71808 (Add long error explanation for E0539) Failed merges: r? @ghost

rust-highfive assigned cramertj May 1, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 1, 2020

rust-highfive assigned oli-obk and unassigned cramertj May 1, 2020

oli-obk reviewed May 1, 2020

View reviewed changes

src/librustc_typeck/check/coercion.rs Show resolved Hide resolved

oli-obk reviewed May 1, 2020

View reviewed changes

src/librustc_typeck/check/demand.rs Outdated Show resolved Hide resolved

oli-obk reviewed May 1, 2020

View reviewed changes

src/test/ui/issues/issue-71676-2.rs Outdated Show resolved Hide resolved

ldm0 force-pushed the ref2ptr branch from 9aa23c7 to a74a048 Compare May 1, 2020 14:03

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 May 1, 2020

Dylan-DPC-zz mentioned this pull request May 1, 2020

Rollup of 5 pull requests #71776

Merged

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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 2, 2020

ldm0 added 4 commits May 2, 2020 12:04

Suggest deref when coercing ty::Ref to ty::RawPtr with arbitrary

60d62be

mutability

Add comments for deref_steps()

80d04cc

Suggestion for immutable reference -> mutable pointer should be emitted

089d4bb

as `Applicability::Unspecified`

Replace convenient function remove_prefix() with replace_prefix()

9a212c1

ldm0 force-pushed the ref2ptr branch from a74a048 to 9a212c1 Compare May 2, 2020 04:05

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 May 2, 2020

Dylan-DPC-zz mentioned this pull request May 2, 2020

Rollup of 5 pull requests #71803

Closed

Dylan-DPC-zz mentioned this pull request May 3, 2020

Rollup of 4 pull requests #71847

Closed

This was referenced May 3, 2020

Rollup of 3 pull requests #71851

Closed

Rollup of 4 pull requests #71853

Merged

bors merged commit 44e678b into rust-lang:master May 3, 2020

ldm0 deleted the ref2ptr branch May 4, 2020 01:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability #71726

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability #71726

ldm0 commented May 1, 2020 •

edited

Loading

rust-highfive commented May 1, 2020

ldm0 commented May 1, 2020

oli-obk May 1, 2020

ldm0 May 1, 2020

oli-obk May 1, 2020

oli-obk commented May 1, 2020

bors commented May 1, 2020

bors commented May 2, 2020

ldm0 commented May 2, 2020

oli-obk commented May 2, 2020

bors commented May 2, 2020

Suggest deref when coercing ty::Ref to ty::RawPtr with arbitrary mutability #71726

Suggest deref when coercing ty::Ref to ty::RawPtr with arbitrary mutability #71726

Conversation

ldm0 commented May 1, 2020 • edited Loading

rust-highfive commented May 1, 2020

ldm0 commented May 1, 2020

oli-obk May 1, 2020

Choose a reason for hiding this comment

ldm0 May 1, 2020

Choose a reason for hiding this comment

oli-obk May 1, 2020

Choose a reason for hiding this comment

oli-obk commented May 1, 2020

bors commented May 1, 2020

bors commented May 2, 2020

ldm0 commented May 2, 2020

oli-obk commented May 2, 2020

bors commented May 2, 2020

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability #71726

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability #71726

ldm0 commented May 1, 2020 •

edited

Loading