A Self-ish Bug

28 June 2018

I recently found a bug in mutagen: The “exchange arguments” mutation was actually ineffective, because the plugin code inserting it looked like the following (simplified):

quote_stmt!(cx, report_coverage($n..$n + 1, &$sym[$flag], $mask);
                let ($a, $b) = if now($n) { ($b, $a) } else { ($a, $b) };)

Spot the bug? There are two statements in the quote! macro call. Only the first one will actually be inserted, though, the second is silently dropped.

Ok, that wouldn’t be too bad. I was in the process of refactoring the code to pull coverage reporting into the mutagen calls (to reduce the amount of code generated), so the report_coverage call was to go away anyway.

Except this bug masked another, more insiduous one: When I refactored, I found that one of the test would no longer compile methods with self arguments, running into Error E0424 (self keyword used in static method). Consider me confused.

The offending test code in question:

#[mutate]
impl ComplexStruct {
    fn interchange_self(self, other: Self) { }
}

The mutation should exchange self and other. Due to the fact that mutagen bakes all mutations in, selecting the active mutation at runtime, we would emit code like the following:

struct ComplexStruct;

impl ComplexStruct {
    fn interchange_self(self, other: Self) {
        let (__self, other) = if ::mutagen::now(1, ..) {
            (other, self)
        } else {
            (self, other)
        }
    }
}

Note the __self: We cannot bind self within a let binding, so we need to replace it with another name, and this is the one we generate. Within the block, we will replace all occurrences of self with __self, btw. but as the method here is empty, this doesn’t happen here.

So if I put the above code in a plain rust file, add an fn main() {} at the end and run rustc, it compiles. My confusion intensified at this point.

In such situation, I usually take a step back to form a theory on what is going on. Since the function invites the “exchange args” mutation and this mutation might in this case modify self, I’ll best confirm that the self parameter is in fact the problem.

Changing the let binding in the mutation to let (_, _) fixes the compilation and cargo expand --test integration shows the expected code:

impl ComplexStruct {
    fn interchange_self(self, other: Self) {
        // some other code removed for brevity
        static __COVERAGE115: [AtomicUsize; 1] = [ATOMIC_USIZE_INIT];
        let (_, _) = if ::mutagen::now(116, &__COVERAGE115[0], 2) {
            (other, self)
        } else {
            (self, other)
        };
    }
}

Could it be that the self isn’t actually changed to a different ident? To avoid making a long story longer, yes it could. Adding an unwrap call confirmed this suspicion: The self_sym symbol was only defined if self was actually mutable.

(That’s why I ought to refactor this code – it’s getting hard to navigate)

With that out of the way, we could finally replace selfs and be on our merry way, correcting the bug while making the code leaner.