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

as *mut _ does not cause by-mut place evaluation, causing an error stating DerefMut is not implemented, but it is #86262

Open
CAD97 opened this issue Jun 13, 2021 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Jun 13, 2021

Given the code:

#![allow(unused_mut)]

use core::mem::ManuallyDrop;

pub struct RefWrapper<'a, T: ?Sized> {
    slot: &'a mut T,
}

impl<'a, T: ?Sized> RefWrapper<'a, T> {
    pub fn into_raw(this: Self) -> *mut T {
        let mut this = ManuallyDrop::new(this);
        // let this = &mut *this;
        this.slot as *mut T
    }
}

The current output is:

error[E0596]: cannot borrow data in a dereference of `ManuallyDrop<RefWrapper<'_, T>>` as mutable
  --> src/lib.rs:13:9
   |
13 |         this.slot as *mut T
   |         ^^^^^^^^^ cannot borrow as mutable
   |
   = help: trait `DerefMut` is required to modify through a dereference, but it is not implemented for `ManuallyDrop<RefWrapper<'_, T>>`

Ideally, this should just work1. Uncommenting the commented line to add an explicit mutable reborrow allows the code to work as intended. But even if it can't work, the error is clearly very unhelpful, as the issue isn't the lack of DerefMut, but that the compiler selected a shared Deref evaluation of the place.

@rustbot modify labels +C-bug +D-incorrect

Footnotes

  1. Importantly, just work and produce a pointer with valid mut provenance. It would be technically correct but misleading to allow &mut T as *mut T behind a shared reference to behave like &mut T as *const T as *mut T. The actually useful behavior is for the fact that as *mut is a by-mut use of the this.slot place to flow back into the selection of whether to evaluate the this.slot place by-ref (via Deref) or by-mut (via DerefMut) the same way other by-mut place usage does.

@CAD97 CAD97 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 13, 2021
@rustbot rustbot added C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Jun 13, 2021
@estebank
Copy link
Contributor

estebank commented Jan 9, 2023

Triage: no change

@CAD97
Copy link
Contributor Author

CAD97 commented Jan 9, 2023

I believe the change implemented in #86647 is the correct fix; namely, considering an as *mut _ use of a place a by-mut usage for the purpose of selecting whether Deref or DerefMut is used in autoborrow evaluation of the place.

For comparison, writing ptr::from_mut(this.slot) (docs instead, compiles as expected.

That PR stalled waiting for lang/compiler confirmation of the change's impact.

Since this does indeed have lang implications,

@rustbot modify labels +T-lang

If we can get team confirmation that #86647 did indeed have the correct approach, I can rebase and resubmit.

@rustbot rustbot added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 9, 2023
@CAD97 CAD97 changed the title Bad error: DerefMut is not implemented, but it is as *mut _ does not cause by-mut place evaluation, causing an error stating DerefMut is not implemented, but it is Jan 9, 2023
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 C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants