Skip to content

Commit

Permalink
Auto merge of #18252 - ShoyuVanilla:issue-15799, r=Veykril
Browse files Browse the repository at this point in the history
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;

https://github.com/rust-lang/rust-analyzer/blob/5982d9c420d0dc90739171829f0d2e9c80d98979/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;

https://github.com/rust-lang/rust-analyzer/blob/5982d9c420d0dc90739171829f0d2e9c80d98979/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 🤔
  • Loading branch information
bors committed Oct 14, 2024
2 parents a672968 + 3eea7af commit b85026b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/tools/rust-analyzer/crates/hir-ty/src/infer/mutability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,25 @@ impl InferenceContext<'_> {
self.infer_mut_expr(index, Mutability::Not);
}
Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
let mut mutability = mutability;
if let Some((f, _)) = self.result.method_resolutions.get_mut(&tgt_expr) {
if mutability == Mutability::Mut {
if let Some(deref_trait) = self
.db
.lang_item(self.table.trait_env.krate, LangItem::DerefMut)
.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
.trait_data(deref_trait)
.method_by_name(&Name::new_symbol_root(sym::deref_mut.clone()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,31 @@ pub unsafe fn foo(a: *mut A) {
//^^^^^ 💡 warn: variable does not need to be mutable
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;
}
}
"#,
);
}
Expand Down

0 comments on commit b85026b

Please sign in to comment.