From d70c914738733e546a0913216f9dd43b6d8b88d3 Mon Sep 17 00:00:00 2001 From: Sei Lisa Date: Fri, 24 May 2024 23:55:23 +0200 Subject: [PATCH] Fix wrong output leading to incorrect tokenization of minus signs When the tree has a unary minus node (NEG) whose child is a product node (*), and the left operand of the product node starts with a minus sign but is not a NEG node, this produced at least two minus signs in sequence without any spaces. Normally, OptSigns hides this problem, but when it is disabled, or when the left factor is a pre-decrement (--V) node, the problem is visible. Fix by creating a function that detects all kinds of leading minus signs, and use it in place of the comparison with NEG. Fixes #31. Reported by @KrsityKu, who also provided a repro. --- lslopt/lsloutput.py | 30 ++++++++++++++++++++---- unit_tests/regression.suite/issue-31.lsl | 20 ++++++++++++++++ unit_tests/regression.suite/issue-31.out | 14 +++++++++++ unit_tests/regression.suite/issue-31.run | 1 + 4 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 unit_tests/regression.suite/issue-31.lsl create mode 100644 unit_tests/regression.suite/issue-31.out create mode 100644 unit_tests/regression.suite/issue-31.run diff --git a/lslopt/lsloutput.py b/lslopt/lsloutput.py index be04419..1fa0d0b 100644 --- a/lslopt/lsloutput.py +++ b/lslopt/lsloutput.py @@ -194,6 +194,20 @@ class outscript(object): return self.symtab[scope][node]['NewName'] return node + def LeadMinus(self, node): + """Tells whether a node starts with a minus sign""" + # TODO: This may add redundant parentheses around --V. + # More exactly, the line marked "!!!". + # They are necessary to prevent wrong output (---v instead of -(--v)) + # but not always are they actually necessary. See regr/issue-31.lsl + return (node.nt == 'NEG' + or node.nt == '--V' # !!! + or not self.optsigns + and node.nt == 'CONST' + and node.t in {'integer', 'float'} + and copysign(1, node.value) == -1 + ) + def OutIndented(self, node): if node.nt != '{}': self.indentlevel += 1 @@ -235,7 +249,9 @@ class outscript(object): if lnt in self.op_priority: if self.op_priority[lnt] < base_pri: lparen = True - elif lnt == 'NEG' and base_pri > self.op_priority['-']: + elif (self.LeadMinus(child[0]) and base_pri > + self.op_priority['-'] + ): lparen = True # This situation has ugly cases due to the strange precedence @@ -261,13 +277,17 @@ class outscript(object): and lnode.nt not in ('~', '!') ): break - if lnode.nt == 'NEG' and base_pri > self.op_priority['-']: + if (self.LeadMinus(lnode) and base_pri > + self.op_priority['-'] + ): lparen = True if rnt in self.op_priority: if self.op_priority[rnt] <= base_pri: rparen = True - elif rnt == 'NEG' and self.op_priority['-'] < base_pri: + elif (self.LeadMinus(child[1]) and self.op_priority['-'] < + base_pri + ): rparen = True # see above elif rnt in ('~', '!'): @@ -278,7 +298,9 @@ class outscript(object): lnode = lnode.ch[0] if lnode.nt not in self.op_priority and lnode.nt not in ('~', '!'): break - if lnode.nt == 'NEG' and base_pri > self.op_priority['-']: + if (self.LeadMinus(lnode) and base_pri > + self.op_priority['-'] + ): # TODO: Improve right operand parenthesis removal # for minus signs. # Shouldn't this look into the RHS node? diff --git a/unit_tests/regression.suite/issue-31.lsl b/unit_tests/regression.suite/issue-31.lsl new file mode 100644 index 0000000..8621d98 --- /dev/null +++ b/unit_tests/regression.suite/issue-31.lsl @@ -0,0 +1,20 @@ +default{timer(){ + integer ia = llGetUnixTime(); + integer ib = llGetUnixTime(); + integer ic = -5; + integer id = ia*2; + integer ie = -ic*ia; + float fa = llGetTime(); + float fb = llGetTime(); + float fc = -5.5; + float fd = fa*2; + float fe = -fc*fa; + // FIXME: This is broken in the output module. + // The output should parenthesize the first two factors in the products, + // just as they are in the source. Fortunately they are equivalent. + // FIXME: The output is not optimal. + // It would suffice to add a space without parenthesizing the --ia, + // just like in the source. + llOwnerSay((string)[(- --ia*ib)*ib, (-++ia*ib)*ib, --ia*ib, ie]); + llOwnerSay((string)[(- --fa*fb)*fb, (-++fa*fb)*fb, --fa*fb, fe]); +}} diff --git a/unit_tests/regression.suite/issue-31.out b/unit_tests/regression.suite/issue-31.out new file mode 100644 index 0000000..a79b821 --- /dev/null +++ b/unit_tests/regression.suite/issue-31.out @@ -0,0 +1,14 @@ +default +{ + timer() + { + integer ia = llGetUnixTime(); + integer ib = llGetUnixTime(); + integer ie = -(-5) * ia; + float fa = llGetTime(); + float fb = llGetTime(); + float fe = -(-5.5) * fa; + llOwnerSay((string)[-(--ia) * ib * ib, -++ia * ib * ib, (--ia) * ib, ie]); + llOwnerSay((string)[-(--fa) * fb * fb, -++fa * fb * fb, (--fa) * fb, fe]); + } +} diff --git a/unit_tests/regression.suite/issue-31.run b/unit_tests/regression.suite/issue-31.run new file mode 100644 index 0000000..22252af --- /dev/null +++ b/unit_tests/regression.suite/issue-31.run @@ -0,0 +1 @@ +main.py -O -optsigns -