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 deref when coercing mutable ty::Ref to ty::RawPtr #71676

Closed
ldm0 opened this issue Apr 29, 2020 · 0 comments · Fixed by #71726
Closed

Suggest deref when coercing mutable ty::Ref to ty::RawPtr #71676

ldm0 opened this issue Apr 29, 2020 · 0 comments · Fixed by #71726
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ldm0
Copy link
Contributor

ldm0 commented Apr 29, 2020

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 mut a = Emm(Foo(Bar(0)));
    let _: *mut u8 = &a; // ERROR
}

This is a missing part of #71540. I think we should be able to suggest &mut ***a here.

@Alexendoo Alexendoo added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 29, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 2, 2020
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)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 3, 2020
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)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 3, 2020
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)
@bors bors closed this as completed in 44e678b May 3, 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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants