On 22/09/2010, at 4:45 AM, Joseph Wright wrote: > On 21/09/2010 14:05, Will Robertson wrote: >> I'm not willing to throw away the infix notation, > > I'd point out that we have an outstanding bug in the infix system with no obvious solution. For other readers, you'll find that > > \bool_if_p:n { ! (\c_true_bool) && \c_false_bool } > > gives the wrong result. I'd half-forgotten about this, oops. I've now studied the code and fixed up the problem in a rudimentary way (that's all I'm good for, really). For those who are interested I'll explain my analysis and solution below. This means I still want the infix notation to stick around, okay? :) But adding the optimised functional notation definitely seems like a good idea to me, as well. * * * The way the boolean infix notation works is all rather neat; a scanner looks ahead at the material to follow and branches accordingly, expanding everything as it goes with \romannumeral. This expanding-as-it-goes is where the problem started. When gathering boolean bits and combining them and expanding, everything works swimmingly with runs like ( 1 && 1 ) || ( 0 || 1 ) as each boolean subexpression can be collapsed linearly. E.g., the above essentially expands to (1) || ( 0 || 1) (0 || 1) (1) When there are negation symbols around, however, we have to be careful about how and when the logic reversal happens. The code said something like (don't follow this too closely) !(...) -> \expandafter \intexpr_if_even_p:n \number \expandafter \CLEANUP \number \expandafter \SCAN \expandafter \BOOL \romannumeral ... which all looks like quite a mess (and I possible transcribed it slightly incorrectly), but the gist of the matter is that the \romannumeral expansion ends up "running away" from the \ifodd, which doesn't get expanded until much later at an incorrect time. Hence explaining why !(TRUE) || FALSE was returning "true": after \romannumeral started inside the parentheses, it didn't stop until after the "|| FALSE" comparison, only after which was the negation applied. You could consider that the || had a higher precedence in this case; it was actually being interpreted as ! ((TRUE)||FALSE) * * * Well, my solution isn'y very elegant, but according to my tests everything is working correctly. Because of the runaway expansion in the design of the process, I couldn't see a way to limit the grouping of the negation, so I did the only other thing I could think of: invent a new grouping system that reverses its inner logic. (I tried adding in extra runaway expansion steps earlier in the process, but I didn't have a good handle on the system then and failed quickly. Perhaps there is a much simpler solution to the problem that what I've implemented!) Previously, |!(...)| expanded to |!| plus the regular code for grouping to process |(...)|, then the result was reversed. Now, the lookahead goes into a different code branch when it sees a group after a negation, and the fundamental logic inside this group is reversed. There is no "extra step" for performing the \intexpr_if_even_p:n reveral, so the logic now groups correctly. That's the idea, anyway. I've expanded the test suite for the boolean logic (it took me a few goes to get the different reversals correct) and I believe it's all working. But I really have to get some other work done today, so I'll have to stop there. Please let me know if you see any problems or areas for simplification in the code. Best regards, -- Will