From e42479756b1116bdabb69f833af4811f7c6ec90f Mon Sep 17 00:00:00 2001 From: Sei Lisa Date: Mon, 13 Nov 2017 03:50:05 +0100 Subject: [PATCH] Fix bug with nested if's; fix missing EXPR wrap. 097c054 introduced a bug that we hadn't caught until now. In some occasions, it could swap nested conditions in such a way that the 'else' of the outer statement was made to belong to the inner one, like this: if (a) if (b) stuff; else stuff; That is of course parsed with the 'else' belonging to if(b). Fix implemented at output time, by detecting 'if(a) stmt; else y;' with stmt being an 'if' without 'else', and wrapping the stmt in {} like this: 'if(a){if(b) x;} else y;'. This has some similarity with parenthesis addition. But the fix has the corner case that, since {} hides visibility of labels, when the inner 'if' has a label as direct child, it can't be swapped lest the label becomes out of scope. So these cases are detected and skipped in the constant folding module. In the case of 'if(cond);', we transform it to 'cond;', but we forgot to wrap the cond in an EXPR node as required. Fixed too. --- lslopt/lslfoldconst.py | 33 ++++++++++++++++++++++++--------- lslopt/lsloutput.py | 10 +++++++++- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/lslopt/lslfoldconst.py b/lslopt/lslfoldconst.py index a7868ca..a22e3a5 100644 --- a/lslopt/lslfoldconst.py +++ b/lslopt/lslfoldconst.py @@ -1664,7 +1664,15 @@ class foldconst(object): if len(child) > 2: self.FoldTree(child, 2) self.FoldStmt(child, 2) - if self.DoesSomething(child[2]): + # Check if it makes sense to swap if and else branches + # (but don't if the else branch is an elseless 'if' + # with a label, as it will be wrapped in {} making it + # become out of scope) + if (self.DoesSomething(child[2]) + and (child[2]['nt'] != 'IF' + or len(child[2]['ch']) == 3 + or child[2]['ch'][1]['nt'] != '@') + ): # Check if we can gain something by negating the # expression. # Swap 'if' and 'else' branch when the condition has @@ -1674,27 +1682,34 @@ class foldconst(object): child[1], child[2] = child[2], child[1] # Swap them if condition is '==' with integer operands if (child[0]['nt'] == '==' - and child[0]['ch'][0]['t'] - == child[0]['ch'][1]['t'] == 'integer' - ): + and child[0]['ch'][0]['t'] + == child[0]['ch'][1]['t'] == 'integer' + ): child[0]['nt'] = '^' child[1], child[2] = child[2], child[1] # Re-test just in case we swapped in the previous check. - if not self.DoesSomething(child[2]): + if (not self.DoesSomething(child[2]) + and child[1]['nt'] != '@'): # no point in "... else ;" - remove else branch del child[2] if not self.DoesSomething(child[1]): # if (X) ; -> X; if len(child) == 2: - parent[index] = child[0] - # It has been promoted to statement. Fold it. + parent[index] = {'nt':'EXPR', 't':child[0]['t'], + 'ch':[child[0]]} + # It has been promoted to statement. Fold it as such. # (Will remove it if SEF) - self.FoldStmt(child, 0) + self.FoldStmt(parent, index) return # If type(X) != Key, then: # if (X) ; else {stuff} -> if (!X) {stuff} - if child[0]['t'] != 'key': + # (being careful with labels again) + if (child[0]['t'] != 'key' + and (child[2]['nt'] != 'IF' + or len(child[2]['ch']) == 3 + or child[2]['ch'][1]['nt'] != '@') + ): # We've already converted all other types to equivalent # comparisons assert child[0]['t'] == 'integer' diff --git a/lslopt/lsloutput.py b/lslopt/lsloutput.py index 461aaaa..e0c2fcc 100644 --- a/lslopt/lsloutput.py +++ b/lslopt/lsloutput.py @@ -384,7 +384,15 @@ class outscript(object): if nt == 'IF': ret = self.dent() while True: - ret += 'if (' + self.OutExpr(child[0]) + ')\n' + self.OutIndented(child[1]) + ret += 'if (' + self.OutExpr(child[0]) + ')\n' + if (len(child) == 3 + and child[1]['nt'] == 'IF' and len(child[1]['ch']) < 3 + ): + ret += self.dent() + '{\n' + ret += self.OutIndented(child[1]) + ret += self.dent() + '}\n' + else: + ret += self.OutIndented(child[1]) if len(child) < 3: return ret if child[2]['nt'] != 'IF':