TMI: Fun with statefulness
Aug. 27th, 2012 06:51 pmToday I worked on Ionmonkey generators again, trying to address the latest round of feedback on my patches. While I didn't get to everything today, satisfyingly, I did find and fix the bug I'd introduced (unknowingly until now) that was causing two test cases to fail "inexplicably".
In the parser, there's a getToken method (on a token stream) and a matchToken method; the first one effectfully pops a token off the stream and returns it, while matchToken returns a boolean denoting whether the right token was found. It turns out matchToken has a side effect: it increments the lookahead in the parser, so that the next time you call getToken, it uses a different code path to inspect the token that got pushed back onto the stream in the last call to matchToken.
The problem is there's also another overloaded instance of getToken that takes a flag argument: in the code that was behaving badly, it was a flag saying "ignore keywords and treat them as identifiers". And the path that executes if you previously called matchToken was ignoring this flag. This broke two test cases that tested that you're allowed to have a function whose name is a keyword. (The call to getToken with the flag was already there, but I introduced the call to matchToken, to check whether there was a '*' after the function keyword; the change I'm implementing is allowing generators to be written with a distinguished syntax, namely declaring them with function* instead of function.)
I don't know whether this is a bug in existing code or whether I just wasn't aware of the protocol. In any case, once I saw what was going on, it wasn't too hard to fix (though the code now looks more awkward to me).
Now I just need to get to the other twelve reviewer comments ;-) It felt really good to finally have all the test cases pass, though.
In the parser, there's a getToken method (on a token stream) and a matchToken method; the first one effectfully pops a token off the stream and returns it, while matchToken returns a boolean denoting whether the right token was found. It turns out matchToken has a side effect: it increments the lookahead in the parser, so that the next time you call getToken, it uses a different code path to inspect the token that got pushed back onto the stream in the last call to matchToken.
The problem is there's also another overloaded instance of getToken that takes a flag argument: in the code that was behaving badly, it was a flag saying "ignore keywords and treat them as identifiers". And the path that executes if you previously called matchToken was ignoring this flag. This broke two test cases that tested that you're allowed to have a function whose name is a keyword. (The call to getToken with the flag was already there, but I introduced the call to matchToken, to check whether there was a '*' after the function keyword; the change I'm implementing is allowing generators to be written with a distinguished syntax, namely declaring them with function* instead of function.)
I don't know whether this is a bug in existing code or whether I just wasn't aware of the protocol. In any case, once I saw what was going on, it wasn't too hard to fix (though the code now looks more awkward to me).
Now I just need to get to the other twelve reviewer comments ;-) It felt really good to finally have all the test cases pass, though.