Fix parsing of logical expressions

[mpeterv]

Currently parse_logical does not pass max_precedence to parse_logical_or_arithmetic, and, as default max_precedence is math.huge, effectively all logical operators have same precedence and are right associative.

Testcase:

$ cd tools
$ ../deps/luajit/usr/local/bin/luajit pflang-compile '0=1 and 0=0 or 0=0'

Result before fix:

return function(P,length)
   do return false end
end

Result after fix:

return function(P,length)
   do return true end
end

Also added a test for this.

[dpino]

Good catch! I will merge it. Thanks for the patch :)

[mpeterv]

I'm sorry, this fix is incorrect. Parser is trying to use different precedence for and and or but in the spec it is said that "Alternation and concatenation have equal precedence and associate left to right.".

The current behaviour is still wrong as they associate right to left. Changing logical_precedence table to set equal precedence for all logical operators seems to fix that.

I just saw the merge update while typing this. Oops.

[mpeterv]

So, the parsing of logical expressions could be simplified a bit. There is also this strange branch which looks incorrect to me - things like tcp port 80 are parsed as primitives already, so all it does is allowing concatenation by juxtaposition, which is mentioned to be invalid in the spec. E.g., currently 1=1 1=1 is parsed as (1=1) and (1=1), while bpf considers that a syntax error.

[dpino]

I think setting equal precedence for 'and' and 'or' logical operators won't fix the correct evaluation of the expression. Since function 'parse_logical' doesn't pass down 'max_precedence' to 'parse_logical_or_arithmetic', 'max_precedence' is always the lowest precedence and expressions are evaluated right-to-left. So whereas 'and' and 'or' should have equal precedence according to the spec, I think your patch is still correct.

With regard to the strange branch you mentioned, initially there was some misunderstanding about how the expression "tcp port 80" should be parsed. It was thought to be parsed as "tcp and port 80" but that proved to be wrong, as "tcp port 80" is itself an operator. There's an issue opened about removing this implicit conjunction #16.

[mpeterv]

Am I correct that should implicit conjunction be removed, the body of that branch should be replaced with something like error("expected a logical operator, got "..op)?