Fix false lint warnings in match arms with multiple patterns

[hirschenberger]

fixing #13866

[alexcrichton]

This seems like a bit of a roundabout method of iterating over a pattern which is already solved by the Visitor. I would expect the solution to be to ensure that all of the bindings are in the used_mut_nodes map. Do you know why the previous bindings in a pattern are missing from this map?

[hirschenberger]

Yes, it was because the visitor was visiting every Pattern of the match arm separately and, looked up if the NodeId of the pattern was in the used_mut_nodes list of the context which was not for the non-matching patterns.
So I had to find the common Ident for the node Ids and look if any of the Ids where used.

[alexcrichton]

That makes sense to me, but the real bug here seems to be that some identifiers in patterns were missing from the used_mut_nodes map, not that we need to alter the visiting/collection strategy.

[hirschenberger]

But to be precise, they are not used, only their common Ident

[lilyball]

I am not particularly familiar with Visitor or used_mut_nodes (or any of this, really). But if it's processing all the patterns for each match arm separately, could it not explicitly look for the ident instead of the node id? All patterns for a match arm need to bind the same variables, so if the node id for one is in used_mut_nodes then the matching identifiers in the other patterns can be assumed to be used as well.

[hirschenberger]

When all patterns in an arm are processed sepatately, they don't know each other and I have to decide to raise an error in each iteration or not.
I see two solutions:

• To collect all Ids for each Ident and check if any is in the used_mut_nodes list.
• To store a used_mut_idents instead of used_mut_nodes in the context.

[alexcrichton]

Can you add a test where x is not used mutable to verify the number of times the lint is printed?

[alexcrichton]

Ok, let's take this strategy for now. Just a few comments and I think this is good to go!