@pnkfelix I've got a branch that stops applying the
unused_mut lint to unreachable code - this mirrors the AST borrowck behaviour.
I'm not sure that the way I'm doing this is super great, but as far as I could gather, the only thing that would indicate that unreachable code had been removed was that the blocks/statements were gone and therefore locals go unused - I've not been able to work out any cases that would break my approach, but I might after stepping away from it for a while.
You mentioned reaching out to stakeholders to see what approach to take here, who did you have in mind? language team? or are there compiler team people who normally take point on lint issues?
hmm, I guess I was thinking of clippy maintainers
but of course clippy is a different beast
How does your branch look w.r.t. the test suite? I assume it breaks next to nothing ... ?
It doesn't break anything. I've added a test case for the issue and everything else passes.
I'm not all that satisfied with the approach it takes, but I couldn't think of anything better. This is the branch.
do you need this condition
index < self.arg_count + 1 ? It seems like the iterator would never hit it, based on the given range?
(I guess I could have posted that as a comment on the commit, heh)
Ah, you're right, I don't.
what happens if you just use the iterator from
(asking out of curiosity, mostly; I don't particularly care/worry about you adding this new method)
If they aren't assigned to, then I won't see statements/terminators with them, and therefore won't be removed from the new set (that is used in the union) and therefore will be considered used - even if they aren't.
I had that initially and it didn't work IIRC.
I don't think my explanation there quite makes sense, but I did try it.
(i've just double checked that it doesn't work with that)
I don't think the approach you've adopted there is all that bad
it'd be good to document the (very different) roles of the two fields of GatherUsedMutsVisitor. E.g.., if I understand correctly, one field is built up inductively, and the other is torn down co-inductively. I'm not 100% sure I'm even using those words quite right
but the main point being: one starts out as the empty set and gets elements added to it, and the other starts as the whole universe of possible values (the set of Locals) and then gets elements removed from it. Right?
Yeah, that's right. I'll add a comment for that. I just wasn't sure if using "not being initialized" as an signal for "use of local is unreachable" would work.
hmm. I imagine we must have some tests of that (uses of uninitialized locals), but it wouldn't hurt to double check quickly how your branch behaves there
Submitted #55758 for this with those changes. I tried a few tests locally but I couldn't think of anything that would misbehave.