Fix string literal argument repetition in grammar (#181)

* Fix string literal argument repetition in grammar

Fixes #180 by no longer allowing an empty string literal to match the `#ExpressionString` context.

* Fix precedence test for sin expression (#182)

Updated test to check for correct precedence in expressions after fixing grammar error. The order-of-operations precedence was broken previously due to the possibility to insert an empty string literal turning a unary operation into a binary one.

Author: g5t

src/grammar/McCommon.g4

@@ -30,7 +30,7 @@ expr
   : '0'                                             #ExpressionZero
   | IntegerLiteral                                  #ExpressionInteger
   | FloatingLiteral                                 #ExpressionFloat
-  | (args+=StringLiteral)*                          #ExpressionString
+  | (args+=StringLiteral)+                          #ExpressionString
   | Identifier '->' expr                            #ExpressionPointerAccess
   | Identifier '.' expr                             #ExpressionStructAccess
   | Identifier '[' expr ']'                         #ExpressionArrayAccess
@@ -116,4 +116,4 @@ Symbol: 'symbol';  // McCode ???? type ????!?!?!
 UnparsedBlock: '%{' (.)*? '%}'; // Used for raw C code blocks and metadata, etc.
 Include: '%include';
 
-Null: 'NULL'; // remove if we switch to underlying C grammar?
+Null: 'NULL'; // remove if we switch to underlying C grammar?

tests/test_expression.py

@@ -373,28 +373,22 @@ def test_parse_Expr(self):
         pi = Expr.id('PI')
         x = Expr.id('x')
         sin = Value('sin', _object=ObjectType.function)
-        # These two expressions should be identical, but details of the Lexer/Parser pick the second
-        equiv_expr = BinaryOp(
-            DataType.undefined, OpStyle.C, '__call__', [sin], [
-                BinaryOp(
-                    DataType.undefined, OpStyle.C, '/', [
-                        BinaryOp(DataType.undefined, OpStyle.C, '*', [
-                            UnaryOp(DataType.undefined, OpStyle.C, '-', pi)
-                        ], [x])
-                    ], [Value.float(2)]
-                )
-            ]
-        )
-        expr = BinaryOp(DataType.undefined, OpStyle.C, '__call__', [sin],[
-            UnaryOp(DataType.undefined, OpStyle.C, '-', [
-                BinaryOp(DataType.undefined, OpStyle.C, '/', [
-                    BinaryOp(DataType.undefined, OpStyle.C, '*', [pi], [x])], [Value.float(2)])
-            ])
+        uop, bop, u, c = UnaryOp, BinaryOp, DataType.undefined, OpStyle.C
+        two = Value.float(2)
+        # These two expressions have identical string representations,
+        # but an error in the grammar caused the Lexer/Parser pick the second.
+        # Now that the error is fixed, the Unary operation of negation has higher
+        # precedence than either the multiplication or division.
+        expr = bop(u, c, '__call__', [sin], [
+            bop(u, c, '/', [bop(u, c, '*', [uop(u, c, '-', pi)], [x])], [two])
+        ])
+        wrong_precedence_expr = bop(u, c, '__call__', [sin], [
+            uop(u, c, '-', [bop(u, c, '/', [bop(u, c, '*', [pi], [x])], [two])])
         ])
 
         self.assertEqual(sin_minus_pi_x_over_2, expr)
         self.assertEqual(str(sin_minus_pi_x_over_2), str(expr))
-        self.assertEqual(str(sin_minus_pi_x_over_2), str(equiv_expr))
+        self.assertEqual(str(sin_minus_pi_x_over_2), str(wrong_precedence_expr))
 
         # One reason that Expr has a list of expressions:
         atan2_y_x = Expr.parse('arctan2(y, x)')
@@ -506,4 +500,4 @@ def test_is_vector(self):
         expr = -ex / BinaryOp(DataType.undefined, OpStyle.C, '__getitem__', [values], Expr.int(0).expr)
         self.assertEqual(f'{expr}', '-ex / values[0]')
 
-        self.assertFalse(expr.is_vector) # because values[0] is a scalar
+        self.assertFalse(expr.is_vector) # because values[0] is a scalar