Repository: pyocd/pyOCD

Issue:
debug sequences: Assignment expression could have return values
Hi,

In LPC800 DFP (Keil.LPC800_DFP.1.10.1.pdsc), there's a debug sequence containing the following statement:
```
                               
            <!-- wait until core has stopped or run in timeout -->
            <control while="((DHCSR_Val = Read32(DHCSR_Addr)) &amp; 0x00020000) == 0" timeout="100000"/>                                                                                                                                                                                                                                                                                                                                  
                               
```

Which caused the following error in pyOCD:

```
0004269 C Unexpected token Token('EQUAL', '=') at line 1, column 13.
Expected one of: 
	* "!="
	* MORETHAN
	* RPAR
	* PLUS
	* LPAR
	* CIRCUMFLEX
	* LESSTHAN
	* "=="
	* "<<"
	* ">>"
	* ">="
	* STAR
	* MINUS
	* "||"
	* VBAR
	* PERCENT
	* AMPERSAND
	* "&&"
	* SLASH
	* "<="
	* QMARK
Previous tokens: [Token('IDENT', 'DHCSR_Val')]


Context: ((DHCSR_Val = Read32(DHCSR_Addr)) & 0x00020000) == 0
            ^
 [__main__]
```

It seems Keil use the same concept like C does, so the above syntax is valid.

Discussion:
flit: @imi415 Could you test this PR if you have time, please? Thanks! 🙏🏽 
imi415: > @imi415 Could you test this PR if you have time, please? Thanks! 🙏🏽

Hi @flit , I have tested this on LPC804M101JHI33, it appears this PR does fixes this issue. Thanks!
flit: ＠imi415 Thanks for testing!

Patch:
diff --git a/pyocd/debug/sequences/sequences.lark b/pyocd/debug/sequences/sequences.lark
index 58a0b5a4..cf333f7b 100644
--- a/pyocd/debug/sequences/sequences.lark
+++ b/pyocd/debug/sequences/sequences.lark
@@ -1,6 +1,6 @@
 // Lark grammar for debug sequence expressions
 //
-// Copyright (c) 2020-2021 Chris Reed
+// Copyright (c) 2020-2023 Chris Reed
 // SPDX-License-Identifier: Apache-2.0
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -29,26 +29,27 @@
 // 11. &&
 // 12. ||
 // 13. ?:
+// 14. assignments: = += -= *= /= %= &= |= ^= <<= >>=
 
 start:                  statement*
 
 ?statement:             decl_stmt ";"?
-    |                   assign_stmt ";"?
     |                   expr_stmt ";"?
 
 // Allow __var declarations with no initialiser expression even though this is disallowed
 // by the specification, for greater compatibility.
 decl_stmt:              "__var" IDENT ["=" expr]
 
-assign_stmt:            IDENT _compound_assign_op expr
-
 // This creates a tree node for expression statements that is easy to identify.
 expr_stmt:              expr
 
 ?expr:                  logical_or_expr
     |                   ternary_expr
+    |                   assign_expr
     |                   STRLIT
 
+assign_expr:            IDENT _compound_assign_op expr
+
 ternary_expr:           expr "?" expr ":" expr
 
 ?logical_or_expr:       logical_and_expr
diff --git a/pyocd/debug/sequences/sequences.py b/pyocd/debug/sequences/sequences.py
index f4b78110..be3c2520 100644
--- a/pyocd/debug/sequences/sequences.py
+++ b/pyocd/debug/sequences/sequences.py
@@ -1,6 +1,6 @@
 # pyOCD debugger
 # Copyright (c) 2020 Arm Limited
-# Copyright (c) 2021-2022 Chris Reed
+# Copyright (c) 2021-2023 Chris Reed
 # SPDX-License-Identifier: Apache-2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -670,7 +670,7 @@ class SemanticChecker:
                 raise DebugSequenceSemanticError(
                         f"line {tree.meta.line}: cannot store a string to variable '{name}'")
 
-        def assign_stmt(self, tree: LarkTree) -> None:
+        def assign_expr(self, tree: LarkTree) -> None:
             # Assigned variable must have been previously declared.
             # TODO disabled until declarations are fully tracked in scopes.
             assert _is_token(tree.children[0], 'IDENT')
@@ -827,7 +827,7 @@ class Interpreter:
 
             self._scope.set(name, value)
 
-        def assign_stmt(self, tree: LarkTree) -> None:
+        def assign_expr(self, tree: LarkTree) -> int:
             values = self.visit_children(tree)
 
             name = values[0].value
@@ -844,6 +844,9 @@ class Interpreter:
 
             self._scope.set(name, value)
 
+            # Return the variable's value as the assignment expression's value.
+            return value
+
         def expr_stmt(self, tree: LarkTree) -> int:
             values = self.visit_children(tree)
             expr_value = values.pop()


Test patch:
diff --git a/test/unit/test_debug_sequences.py b/test/unit/test_debug_sequences.py
index 37db5a60..0e0b3fe2 100644
--- a/test/unit/test_debug_sequences.py
+++ b/test/unit/test_debug_sequences.py
@@ -1,6 +1,6 @@
 # pyOCD debugger
 # Copyright (c) 2020 Arm Limited
-# Copyright (c) 2021-2022 Chris Reed
+# Copyright (c) 2021-2023 Chris Reed
 # SPDX-License-Identifier: Apache-2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -128,7 +128,7 @@ def context(session, delegate):
     return c
 
 @pytest.fixture(scope='function')
-def block_context(context):
+def block_context(context: DebugSequenceExecutionContext):
     # Return a context set up to be able to run a Block.
     #
     # First create a sequence node with no children. Use the private _create_scope() method to get
@@ -272,7 +272,7 @@ class TestDebugSequenceParser:
     def test_assign_minus_negative(self):
         ast = Parser().parse("a = 1 - -1;")
         print("ast=", ast)
-        e = ast.children[0].children[2] # start.assign_stmt.binary_expr
+        e = ast.children[0].children[0].children[2] # start.expr_stmt.assign_expr.binary_expr
         assert e.children[0] == 1
         assert e.children[1] == LarkToken('MINUS', '-')
         assert e.children[2].data == 'unary_expr'
@@ -282,7 +282,7 @@ class TestDebugSequenceParser:
     def test_assign_minus_positive(self):
         ast = Parser().parse("a = 1 - +1;")
         print("ast=", ast)
-        e = ast.children[0].children[2] # start.assign_stmt.binary_expr
+        e = ast.children[0].children[0].children[2] # start.expr_stmt.assign_expr.binary_expr
         assert e.children[0] == 1
         assert e.children[1] == LarkToken('MINUS', '-')
         assert e.children[2].data == 'unary_expr'
@@ -298,6 +298,51 @@ class TestDebugSequenceParser:
         print("ast=", ast)
         assert ast.children[0].children[0] == 1000
 
+    def test_simple_assign_expr(self):
+        ast = Parser().parse("x = 10;")
+        print("ast=", ast)
+        e = ast.children[0].children[0] # start.expr_stmt.assign_expr
+        assert e.children[2] == 10
+
+    def test_multi_assign_expr(self):
+        # ast= start
+        # expr_stmt
+        #     assign_expr
+        #     x
+        #     =
+        #     binary_expr
+        #         assign_expr
+        #         y
+        #         +=
+        #         binary_expr
+        #             1
+        #             +
+        #             1
+        #         +
+        #         2
+        ast = Parser().parse("x = (y += (1 + 1)) + 2;")
+        print("ast=", ast.pretty())
+        e = ast.children[0].children[0] # start.expr_stmt.assign_expr
+        assert e.data == LarkToken('RULE', 'assign_expr')
+        assert e.children[0] == 'x'
+        e = e.children[2].children[0]
+        assert e.data == LarkToken('RULE', 'assign_expr')
+        assert e.children[0] == 'y'
+        assert e.children[1] == "+="
+
+    def test_assign_expr_precedence_1(self):
+        ast = Parser().parse("x = 1 ? 2 : 3;")
+        print("ast=", ast.pretty())
+        e = ast.children[0].children[0] # start.expr_stmt.assign_expr
+        assert e.data == LarkToken('RULE', 'assign_expr')
+
+    def test_assign_expr_precedence_2(self):
+        ast = Parser().parse("1 ? x = 2 : 3;")
+        print("ast=", ast.pretty())
+        e = ast.children[0].children[0] # start.expr_stmt.assign_expr
+        assert e.data == LarkToken('RULE', 'ternary_expr')
+        assert e.children[1].data == LarkToken('RULE', 'assign_expr')
+
 class TestDebugSequenceBlockExecute:
     def test_semicolons(self, block_context):
         block_context.current_scope.set("a", 0)
@@ -440,7 +485,7 @@ class TestDebugSequenceBlockExecute:
         actual = block_context.current_scope.get("x")
         assert actual == result
 
-    def test_unary_op_assign(self, block_context):
+    def test_unary_op_assign_block(self, block_context):
         # Statement from Infineon.PSoC6_DFP
         s = Block("__Result = -1; // DAP is unavailable")
         logging.info(f"Block: {s._ast.pretty()}")
@@ -477,6 +522,13 @@ class TestDebugSequenceBlockExecute:
         s.execute(block_context)
         assert block_context.current_scope.get("x") == result
 
+    def test_assign_expr_block(self, block_context):
+        s = Block("__var x; __var y; x = 1 + (y = 2);")
+        logging.info(f"Block: {s._ast.pretty()}")
+        s.execute(block_context)
+        assert block_context.current_scope.get("x") == 3
+        assert block_context.current_scope.get("y") == 2
+
 class TestConstantFolder:
     def _get_folded_ast(self, expr):
         ast = Parser.parse(expr)
@@ -710,4 +762,12 @@ class TestDebugSequences:
         seq.add_child(w)
         seq.execute(context)
 
+    def test_exec_while_with_assign(self, context: DebugSequenceExecutionContext):
+        seq = DebugSequence('test')
+        seq.add_child(Block("__var x = 0; __var y = 0;"))
+        w = WhileControl("x < (y = 2)")
+        w.add_child(Block("x += 1;"))
+        seq.add_child(w)
+        seq.execute(context)
+