Auto merge of #18252 - ShoyuVanilla:issue-15799, r=Veykril

fix: Do not consider mutable usage of deref to `*mut T` as deref_mut

Fixes #15799

We are doing some heuristics for deciding whether the given deref is deref or deref_mut here;

5982d9c420/crates/hir-ty/src/infer/mutability.rs (L182-L200)

But this heuristic is erroneous if we are dereferencing to a mut ptr and normally those cases are filtered out here as builtin;

5982d9c420/crates/hir-ty/src/mir/lower/as_place.rs (L165-L177)

Howerver, this works not so well if the given dereferencing is double dereferencings like the case in the #15799.

```rust
struct WrapPtr(*mut u32);

impl core::ops::Deref for WrapPtr {
    type Target = *mut u32;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

fn main() {
    let mut x = 0u32;
    let wrap = WrapPtr(&mut x);
    unsafe {
        **wrap = 6;
    }
}
```

Here are two - outer and inner - dereferences here, and the outer dereference is marked as deref_mut because there is an assignment operation.
And this deref_mut marking is propagated into the inner dereferencing.
In the later MIR lowering, the outer dereference is filtered out as it's expr type is `*mut u32`, but the expr type in the inner dereference is an ADT, so this false-mutablility is not filtered out.

This PR cuts propagation of this false mutablilty chain if the expr type is mut ptr.
Since this happens before the resolve_all, it may have some limitations when the expr type is determined as mut ptr at the very end of inferencing, but I couldn't find simple fix for it 🤔
This commit is contained in:
bors 2024-10-14 12:07:31 +00:00
commit 9f1f5cd8f6
2 changed files with 37 additions and 1 deletions

View file

@ -180,6 +180,7 @@ impl InferenceContext<'_> {
self.infer_mut_expr(index, Mutability::Not); self.infer_mut_expr(index, Mutability::Not);
} }
Expr::UnaryOp { expr, op: UnaryOp::Deref } => { Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
let mut mutability = mutability;
if let Some((f, _)) = self.result.method_resolutions.get_mut(&tgt_expr) { if let Some((f, _)) = self.result.method_resolutions.get_mut(&tgt_expr) {
if mutability == Mutability::Mut { if mutability == Mutability::Mut {
if let Some(deref_trait) = self if let Some(deref_trait) = self
@ -187,7 +188,17 @@ impl InferenceContext<'_> {
.lang_item(self.table.trait_env.krate, LangItem::DerefMut) .lang_item(self.table.trait_env.krate, LangItem::DerefMut)
.and_then(|l| l.as_trait()) .and_then(|l| l.as_trait())
{ {
if let Some(deref_fn) = self let ty = self.result.type_of_expr.get(*expr);
let is_mut_ptr = ty.is_some_and(|ty| {
let ty = self.table.resolve_ty_shallow(ty);
matches!(
ty.kind(Interner),
chalk_ir::TyKind::Raw(Mutability::Mut, _)
)
});
if is_mut_ptr {
mutability = Mutability::Not;
} else if let Some(deref_fn) = self
.db .db
.trait_data(deref_trait) .trait_data(deref_trait)
.method_by_name(&Name::new_symbol_root(sym::deref_mut.clone())) .method_by_name(&Name::new_symbol_root(sym::deref_mut.clone()))

View file

@ -1255,6 +1255,31 @@ pub unsafe fn foo(a: *mut A) {
//^^^^^ 💡 warn: variable does not need to be mutable //^^^^^ 💡 warn: variable does not need to be mutable
let _ = b(); let _ = b();
} }
"#,
);
}
#[test]
fn regression_15799() {
check_diagnostics(
r#"
//- minicore: deref_mut
struct WrapPtr(*mut u32);
impl core::ops::Deref for WrapPtr {
type Target = *mut u32;
fn deref(&self) -> &Self::Target {
&self.0
}
}
fn main() {
let mut x = 0u32;
let wrap = WrapPtr(&mut x);
unsafe {
**wrap = 6;
}
}
"#, "#,
); );
} }