From 274a758332f11c78fb78da5d8b1c256bf30366d3 Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 19 Aug 2019 14:32:59 -0500 Subject: [PATCH 1/2] Stringifier: do not assume associativity of multiplicative (*,/,%,//) operators, add tests --- pymbolic/mapper/stringifier.py | 80 ++++++++++++++++++++++------------ test/test_pymbolic.py | 60 ++++++++++++++++--------- 2 files changed, 92 insertions(+), 48 deletions(-) diff --git a/pymbolic/mapper/stringifier.py b/pymbolic/mapper/stringifier.py index c5abb3a..a3713a6 100644 --- a/pymbolic/mapper/stringifier.py +++ b/pymbolic/mapper/stringifier.py @@ -23,6 +23,7 @@ THE SOFTWARE. """ import pymbolic.mapper +import pymbolic.primitives as p __doc__ = """ .. _prec-constants: @@ -101,10 +102,21 @@ class StringifyMapper(pymbolic.mapper.Mapper): def join(self, joiner, iterable): return self.format(joiner.join("%s" for i in iterable), *iterable) + def rec_with_force_parens_around(self, expr, *args, **kwargs): + force_parens_around = kwargs.pop("force_parens_around", ()) + + result = self.rec(expr, *args, **kwargs) + + if isinstance(expr, force_parens_around): + result = "(%s)" % result + + return result + def join_rec(self, joiner, iterable, prec, *args, **kwargs): f = joiner.join("%s" for i in iterable) return self.format(f, - *[self.rec(i, prec, *args, **kwargs) for i in iterable]) + *[self.rec_with_force_parens_around(i, prec, *args, **kwargs) + for i in iterable]) def parenthesize(self, s): return "(%s)" % s @@ -183,9 +195,14 @@ class StringifyMapper(pymbolic.mapper.Mapper): self.join_rec(" + ", expr.children, PREC_SUM, *args, **kwargs), enclosing_prec, PREC_SUM) + # {{{ multiplicative operators + + multiplicative_primitives = (p.Product, p.Quotient, p.FloorDiv, p.Remainder) + def map_product(self, expr, enclosing_prec, *args, **kwargs): return self.parenthesize_if_needed( - self.join_rec("*", expr.children, PREC_PRODUCT, *args, **kwargs), + self.join_rec("*", expr.children, PREC_PRODUCT, *args, **kwargs, + force_parens_around=(p.Quotient, p.FloorDiv, p.Remainder)), enclosing_prec, PREC_PRODUCT) def map_quotient(self, expr, enclosing_prec, *args, **kwargs): @@ -193,27 +210,43 @@ class StringifyMapper(pymbolic.mapper.Mapper): self.format("%s / %s", # space is necessary--otherwise '/*' becomes # start-of-comment in C. ('*' from dereference) - self.rec(expr.numerator, PREC_PRODUCT, *args, **kwargs), - self.rec( - expr.denominator, PREC_POWER, # analogous to ^{-1} - *args, **kwargs)), + self.rec_with_force_parens_around(expr.numerator, PREC_PRODUCT, + *args, + force_parens_around=self.multiplicative_primitives, + **kwargs), + self.rec_with_force_parens_around( + expr.denominator, PREC_PRODUCT, *args, + force_parens_around=self.multiplicative_primitives, + **kwargs)), enclosing_prec, PREC_PRODUCT) def map_floor_div(self, expr, enclosing_prec, *args, **kwargs): - # (-1) * ((-1)*x // 5) should not reassociate. Therefore raise precedence - # on the numerator and shield against surrounding products. - - result = self.format("%s // %s", - self.rec(expr.numerator, PREC_POWER, *args, **kwargs), - self.rec( - expr.denominator, PREC_POWER, # analogous to ^{-1} - *args, **kwargs)) - - # Note ">=", not ">" as in parenthesize_if_needed(). - if enclosing_prec >= PREC_PRODUCT: - return "(%s)" % result - else: - return result + return self.parenthesize_if_needed( + self.format("%s // %s", + self.rec_with_force_parens_around( + expr.numerator, PREC_PRODUCT, *args, + force_parens_around=self.multiplicative_primitives, + **kwargs), + self.rec_with_force_parens_around( + expr.denominator, PREC_PRODUCT, *args, + force_parens_around=self.multiplicative_primitives, + **kwargs)), + enclosing_prec, PREC_PRODUCT) + + def map_remainder(self, expr, enclosing_prec, *args, **kwargs): + return self.parenthesize_if_needed( + self.format("%s %% %s", + self.rec_with_force_parens_around( + expr.numerator, PREC_PRODUCT, *args, + force_parens_around=self.multiplicative_primitives, + **kwargs), + self.rec_with_force_parens_around( + expr.denominator, PREC_PRODUCT, *args, + force_parens_around=self.multiplicative_primitives, + **kwargs)), + enclosing_prec, PREC_PRODUCT) + + # }}} def map_power(self, expr, enclosing_prec, *args, **kwargs): return self.parenthesize_if_needed( @@ -222,13 +255,6 @@ class StringifyMapper(pymbolic.mapper.Mapper): self.rec(expr.exponent, PREC_POWER, *args, **kwargs)), enclosing_prec, PREC_POWER) - def map_remainder(self, expr, enclosing_prec, *args, **kwargs): - return self.format("(%s %% %s)", - self.rec(expr.numerator, PREC_PRODUCT, *args, **kwargs), - self.rec( - expr.denominator, PREC_POWER, # analogous to ^{-1} - *args, **kwargs)) - def map_polynomial(self, expr, enclosing_prec, *args, **kwargs): from pymbolic.primitives import flattened_sum return self.rec(flattened_sum( diff --git a/test/test_pymbolic.py b/test/test_pymbolic.py index 0bd708c..89b23ef 100644 --- a/test/test_pymbolic.py +++ b/test/test_pymbolic.py @@ -36,6 +36,33 @@ except NameError: from functools import reduce +# {{{ utilities + +def assert_parsed_same_as_python(expr_str): + # makes sure that has only one line + expr_str, = expr_str.split('\n') + from pymbolic.interop.ast import ASTToPymbolic + import ast + ast2p = ASTToPymbolic() + try: + expr_parsed_by_python = ast2p(ast.parse(expr_str).body[0].value) + except SyntaxError: + with pytest.raises(ParseError): + parse(expr_str) + else: + expr_parsed_by_pymbolic = parse(expr_str) + assert expr_parsed_by_python == expr_parsed_by_pymbolic + + +def assert_parse_roundtrip(expr_str): + expr = parse(expr_str) + from pymbolic.mapper.stringifier import StringifyMapper + strified = StringifyMapper()(expr) + assert strified == expr_str, (strified, expr_str) + +# }}} + + def test_integer_power(): from pymbolic.algorithm import integer_power @@ -222,27 +249,6 @@ def test_parser(): print(repr(parse(":5:1"))) print(repr(parse("3:5:1"))) - def assert_parse_roundtrip(expr_str): - expr = parse(expr_str) - from pymbolic.mapper.stringifier import StringifyMapper - strified = StringifyMapper()(expr) - assert strified == expr_str, (strified, expr_str) - - def assert_parsed_same_as_python(expr_str): - # makes sure that has only one line - expr_str, = expr_str.split('\n') - from pymbolic.interop.ast import ASTToPymbolic - import ast - ast2p = ASTToPymbolic() - try: - expr_parsed_by_python = ast2p(ast.parse(expr_str).body[0].value) - except SyntaxError: - with pytest.raises(ParseError): - parse(expr_str) - else: - expr_parsed_by_pymbolic = parse(expr_str) - assert expr_parsed_by_python == expr_parsed_by_pymbolic - assert_parse_roundtrip("()") assert_parse_roundtrip("(3,)") @@ -623,6 +629,18 @@ def test_make_sym_vector(): assert len(make_sym_vector("vec", [1, 2, 3])) == 3 +def test_multiplicative_stringify_preserves_association(): + for inner in ["*", " / ", " // ", " % "]: + for outer in ["*", " / ", " // ", " % "]: + if outer == inner: + continue + + assert_parse_roundtrip("x%s(y%sz)" % (outer, inner)) + assert_parse_roundtrip("(y%sz)%sx" % (inner, outer)) + + assert_parse_roundtrip("(-1)*(((-1)*x) / 5)") + + if __name__ == "__main__": import sys if len(sys.argv) > 1: -- GitLab From dce5e9855c5d37ec30d3a5d7d6cdf9c8d2944e7e Mon Sep 17 00:00:00 2001 From: Andreas Kloeckner Date: Mon, 19 Aug 2019 14:54:45 -0500 Subject: [PATCH 2/2] Fix for Py2 syntax --- pymbolic/mapper/stringifier.py | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/pymbolic/mapper/stringifier.py b/pymbolic/mapper/stringifier.py index a3713a6..da65138 100644 --- a/pymbolic/mapper/stringifier.py +++ b/pymbolic/mapper/stringifier.py @@ -200,50 +200,41 @@ class StringifyMapper(pymbolic.mapper.Mapper): multiplicative_primitives = (p.Product, p.Quotient, p.FloorDiv, p.Remainder) def map_product(self, expr, enclosing_prec, *args, **kwargs): + kwargs["force_parens_around"] = (p.Quotient, p.FloorDiv, p.Remainder) return self.parenthesize_if_needed( - self.join_rec("*", expr.children, PREC_PRODUCT, *args, **kwargs, - force_parens_around=(p.Quotient, p.FloorDiv, p.Remainder)), + self.join_rec("*", expr.children, PREC_PRODUCT, *args, **kwargs), enclosing_prec, PREC_PRODUCT) def map_quotient(self, expr, enclosing_prec, *args, **kwargs): + kwargs["force_parens_around"] = self.multiplicative_primitives return self.parenthesize_if_needed( self.format("%s / %s", # space is necessary--otherwise '/*' becomes # start-of-comment in C. ('*' from dereference) self.rec_with_force_parens_around(expr.numerator, PREC_PRODUCT, - *args, - force_parens_around=self.multiplicative_primitives, - **kwargs), + *args, **kwargs), self.rec_with_force_parens_around( - expr.denominator, PREC_PRODUCT, *args, - force_parens_around=self.multiplicative_primitives, - **kwargs)), + expr.denominator, PREC_PRODUCT, *args, **kwargs)), enclosing_prec, PREC_PRODUCT) def map_floor_div(self, expr, enclosing_prec, *args, **kwargs): + kwargs["force_parens_around"] = self.multiplicative_primitives return self.parenthesize_if_needed( self.format("%s // %s", self.rec_with_force_parens_around( - expr.numerator, PREC_PRODUCT, *args, - force_parens_around=self.multiplicative_primitives, - **kwargs), + expr.numerator, PREC_PRODUCT, *args, **kwargs), self.rec_with_force_parens_around( - expr.denominator, PREC_PRODUCT, *args, - force_parens_around=self.multiplicative_primitives, - **kwargs)), + expr.denominator, PREC_PRODUCT, *args, **kwargs)), enclosing_prec, PREC_PRODUCT) def map_remainder(self, expr, enclosing_prec, *args, **kwargs): + kwargs["force_parens_around"] = self.multiplicative_primitives return self.parenthesize_if_needed( self.format("%s %% %s", self.rec_with_force_parens_around( - expr.numerator, PREC_PRODUCT, *args, - force_parens_around=self.multiplicative_primitives, - **kwargs), + expr.numerator, PREC_PRODUCT, *args, **kwargs), self.rec_with_force_parens_around( - expr.denominator, PREC_PRODUCT, *args, - force_parens_around=self.multiplicative_primitives, - **kwargs)), + expr.denominator, PREC_PRODUCT, *args, **kwargs)), enclosing_prec, PREC_PRODUCT) # }}} -- GitLab