&mut &mut T about rust-clippy HOT 13 CLOSED

What does that even mean? A mutable pointer to a mutable pointer?

It'd be easy to match but what should clippy suggest (apart from a visit to a good psychiatrist)?

from rust-clippy.

&mut T I guess? It's really hard to tell what the person actually wanted, you just know it probably wasn't that.

from rust-clippy.

Ok, so we assume that &mut &mut T is a copy'n'paste mistake, and the dev would want any of &mut &T, & &mut T or &mut T.

&mut is matched by either Ty{node: TyPtr(MutTy{ty: ref pty, mutbl:MutMutable}), ..} or Ty{node: TyRPtr(_, MutTy{ty: ref pty, mutbl:MutMutable}), ..}, where the ref pty is the reference. So if we have a fn match_mut(&Ty) -> Option(&Ty) that will do the above match and returnptyorNone, we can checkmatch_mut(ty).and_then(|ty| match_mut(ty)).is_some()(we need the closure to auto-deref theptr::P`) to find the pattern.

from rust-clippy.

I have a half-baked version in my tree, but it doesn't catch all cases. Simply using check_ty doesn't cut it, and there are all kinds of expressions where we need to unpack Tys.

from rust-clippy.

Uh, just unwrap the sty with ty_ptr of the ty::Ty coming out of expr_ty? in check_expr?

from rust-clippy.

I already did that, but it failed to catch

const this_is_deranged : &mut &mut u32 = &mut &mut 1u32;

from rust-clippy.

Why? The RHS is an expr which should be a ty_ptr(ty_ptr(ty_int)) or something

from rust-clippy.

In fact, it's an ExprAddrOf(MutMutable, ..ExprAddrOf(MutMutable, _)). And I found it.

from rust-clippy.

Ah, you misunderstand me.

While we do want to catch types which are &mut &mut Foo or expressions which are &mut &mut foo; that's not all we want to do. Those are pure AST checks.

We also want to catch cases where an &mut ptr is being needlessly re-&mut-ed. This is done by using expr_ty on an expression and checking its actual type. (The type info isn't part of the AST, it's part of the tcx)

To make this robust, I would suggest, in check_expr:

• If it's &mut _, run expr_ty on the inner thingy and make sure it's not a mutable ty_ptr. If it is, then lint that reborrowing might be preferable.
• If it's &mut &mut, do as we do now
• If it's neither, run expr_ty on the whole expr and check that it's not a double mutable ptr (and warn if so)

from rust-clippy.

(So thanks for the lint ❤️, but there's more work to be done to close this issue 😄 )

from rust-clippy.

Ah, thank you for the clarification. I will extend the lint as you suggested.

from rust-clippy.

Thanks!

from rust-clippy.

And...done (At least I hope so).

from rust-clippy.