Variables that are only used for swapping are marked as unused

[marijnh]

This causes a warning that x is unused:

fn main() {
   let x = 10, y = 20;
   x <-> y;
   log(error, y);
}

[catamorphism]

Definitely a bug; will be annoying to fix, though, since this is the only thing that makes the "unused variables" analysis non-local.

[marijnh]

How does it make it non-local? If you just perform the same marking of used for the LHS of the swap as you're doing for the RHS, I think that's plenty good enough.

[catamorphism]

Well, the question is, suppose I have the following program:

fn main() {
   let x = 10, y = 20;
   x <-> y;

Where x and y get swapped, but never used. If I treat both sides of a swap the same way I would treat the RHS of an assignment, then the compiler won't print a warning for this program. Arguably that's wrong -- x and y are both swapped, but neither is actually, used.

OTOH, if others think that a conservative approximation (not warning for the above case) is acceptable, I'm ok with doing the simplest thing. Does that make sense?

[marijnh]

A conservative thing that at least doesn't emit spurious warnings is good enough for me. A 'correct' analysis would be awesome, but probably not worth the extra complexity.

[catamorphism]

I'm ok with doing the easy thing here :-)

[jruderman]

IMO, the "unused" warning should be per-assignment, not per-variable.

fn main() {
   let x = 10;
   x = 20;
   log(error, x);
}

should warn because the first assignment to x is unused.

fn main() {
   let x = 10;
   log(error, x);
   x = 20;
}

should warn because the second assignment to x is unused. Finally, the original example,

fn main() {
   let x = 10, y = 20;
   x <-> y;
   log(error, y);
}

should warn because the second assignment to x is unused.

[marijnh]

I agree, actually.

The last-use pass already has this information, and is probably easier to extend than the typestate code. We could move unused variable analysis into that pass.

[catamorphism]

The original bug seems to have been fixed. I don't know how to add a test case for it, because there doesn't seem to be a way (in lint) to turn the unused-variable warning into an error.

As for @jruderman 's proposal, I'm going to turn that into a separate bug.