Ticket #38: multiArg-comma-sep.patch

Source/Statements.cobra

@@ -1383 +1383 @@
             # TODO: can there just be "yield return"?
             pass
         curCodeMember.hasYieldStmt = true
-
-
+        
 class MultiTargetAssignStatement
     is partial
     inherits Stmt
     """
-    Handle atatement of form 
-        a, b[,...] = <List>
-    Gets turned into a sequence of assignExpr.
-        a = <List>[0]
-        b = <List>[1]
-
+    Handle statements of form 
+        a,[b,...] = <List-Generating-Expr>
+    or
+        a,[b,...] = <expr>,[<expr>,...]
+    Gets turned into various sequences of assignExpr.
+        tmp = <List-Generating-Expr>
+        a = tmp[0]
+        b = tmp[1]
+        ...
+        a = <expr>
+        b = <expr>
+        ...
     TODO: support a, b[,...] = <Enumerable> 
     TODO: support a, b[,...] = <KeyValuePair|DictionaryEntry>
-    TODO: support a, b[,...] = c,d[,...] form
     """
+
     var _targets as List<of Expr> # Lvalues
-    var _source as Expr         # evaluate to IList/Array/String or otherwise indexable by int
-    var _block as BlockStmt?    # the rewritten/expanded code
+    var _source as Expr?          # evaluate to IList/Array/String or otherwise indexable by int
+    var _rvals as List<of Expr>?  # list of source expressions
+    var _block as BlockStmt?      # the rewritten/expanded code
     
-    def init(opToken as IToken, args as List<of Expr>, rhs as Expr)
+    def init(opToken as IToken, args as List<of Expr>, rhs as Expr?, rhsList as List<of Expr>?)
         base.init(opToken)
         assert opToken.which == 'ASSIGN'
+        assert (rhs or rhsList) and not (rhs and rhsList) # either but not both
         _targets = args
         _source = rhs
+        _rvals = rhsList
         
     def addSubFields
         base.addSubFields
         .addField('targets', _targets)
         .addField('source', _source)
+        .addField('rvals', _rvals)
         .addField('block', _block)
 
     get targets from var
 
     get source from var
 
+    get rvals from var
+
     get block from var
 
-    def _writeBlock
+    def _bindImp
+        assert _source or _rvals
+        base._bindImp
+        for target in _targets
+            if not _isLValue(target)
+                .throwError('"[target.toCobraSource]" is not an assignable lvalue (identifier, var, property or indexer).')
+        if _rvals
+            if _targets.count <> _rvals.count
+                .throwError('The number of targets ([_targets.count]) must be the same as the number of expressions ([_rvals.count]) assigned to them')
+            #If rvals are all safe to assign (no rerefs) we can do the simplest thing and turn 
+            # them into a sequence of individual assignments.   i.e a,b = 1,2  or a,b = c,d   cases
+            # If not, we need to turn the rvals list into a literal list, pinning the current values,
+            # and do the expansion on the list as a single item  i.e  a,b = b,a -> a,b = [b,a] 
+            if _isSafeToSimpleAssign()
+                _writeBlockForList
+            else
+                _source = ListLit(.lastToken.copy('LBRACKET', r'['), _rvals)
+        if _source
+            #TODO: chk _source supports an int indexer  somehow...
+            #if not .can_be_indexed_by_int_indexer(_source) 
+            #   .throwError(r"rhs expression [_source.toCobraSource] must support being indexed by an integer offset (e.g rhs[0]")
+            _writeBlockForItem
+                
+        _block.bindImp
+        
+    def _isLValue(id as Expr) as bool
+        # TODO: push to Expr.isLValue. probably require .didBindImp
+        # TODO: for DotExpr and IndexExpr check if there is a setter
+        # TODO: for MemberExpr check that its a setter property or visible var (not a method)
+        return id inherits IdentifierExpr or id inherits DotExpr or id inherits IndexExpr
+        
+    def _isSafeToSimpleAssign as bool
+        # conservatively we just presume its safe if the rvals list is all Literals
+        if all for e in _rvals get e inherits Literal
+            return true 
+        # or if identifiers in rvals are not also in target
+        if _disjointIdentifierLists() 
+            return true
+        # There are possibly others but above expected to be most common usage  
+        # TODO: expand this for more wide ranging simple assignment e.g MemberExprs non matching
+        return false
+
+    def _disjointIdentifierLists as bool
+        # Specifically all of targets list are IdentifierExprs and rvals are Literals, DotExprs, 
+        # MemberExprs or Indentifiers and any Identifiers are not also in target list
+        tnames = Set<of String>()
+        for t in _targets
+            if t inherits IdentifierExpr or t inherits AsExpr
+                tnames.add((t to NameExpr).name)    
+            else
+                return false
+        if all for e in _rvals get e inherits Literal or e inherits DotExpr or e inherits MemberExpr or _
+                (e inherits IdentifierExpr and (e to IdentifierExpr).name not in tnames) 
+            return true 
+        return false
+        
+    def _writeBlockForList
         """
-        Construct expanded assignment block for a multi target assignment.
+        Construct expanded assignment block for a multi target assignment from a list of expressions.
+        Assignment block contains
+            id0 = rvals[0] # id0 is contents of targets[0]
+            id1 = rvals[1] # id1 is contents of targets[1]
+                (... repeated for number of items in targets specifying ids (lvalues))
+        """
+        assert _targets.count == _rvals.count
+        stmts = List<of Stmt>()
+        assignTkn = .lastToken  # the one we stored in init
+        count = 0
+        for id in _targets
+            expr = _rvals[count]
+            stmts.add(AssignExpr(assignTkn, assignTkn.which, id, expr))
+            count += 1
+        _block = BlockStmt(.lastToken.copy('INDENT', ''), stmts)    
+
+    def _writeBlockForItem
+        """
+        Construct expanded assignment block for a multi target assignment from a single source expression.
         if fewer targets than exprs in _source - extra exprs (silently) ignored
         TODO: give error when rvalues are too many
         if more targets than exprs in _source   - will emit runtime exception
             above differ fm python - both give errors
         Unchecked assumptions; 
-            source is int indexable (x[n]) - c# compiler pick up
+            source is int indexable (x[n]) - c# compiler pick up currently
         Assignment block contains
             lh_mt_serno = <source>
             id0 = lh_mt_serno[0] # id0 is contents of targets[0]
@@ -1459 +1544 @@
             stmts.add(AssignExpr(assignTkn, assignTkn.which, id, idxExpr))
             count += 1
         _block = BlockStmt(ttoken.copy('INDENT', ''), stmts)    
-
-    def _isLValue(id as Expr) as bool
-        # TODO: push to Expr.isLValue. probably require .didBindImp
-        # TODO: for DotExpr and IndexExpr check if there is a setter
-        # TODO: for MemberExpr check that its a setter property or visible var (not a method)
-        return id inherits IdentifierExpr or id inherits DotExpr or id inherits IndexExpr
-
-    def _bindImp
-        base._bindImp
-        for target in _targets
-            if not _isLValue(target)
-                .throwError('"[target.toCobraSource]" is not an assignable lvalue (identifier, var, property or indexer).')
-        # TODO: check _source supports an int indexer ??
-        _writeBlock
-        _block.bindImp

Source/CobraParser.cobra

@@ -2718 +2718 @@
         if .last.which <> 'ASSIGN'
             .throwError('Comma-separated assignment targets must end with "=", or this is a general syntax error.')
         assignTkn = .last to !
-        rhs = .expression
-        return MultiTargetAssignStatement(assignTkn, args, rhs)
+        rhs as Expr? = .expression
+        if .optional('COMMA')
+            rhsList = .commaSepExprs('EOL')
+            assert .last.which=='EOL'
+            .undo  # need EOL
+            rhsList.insert(0, rhs)
+            rhs = nil
+        return MultiTargetAssignStatement(assignTkn, args, rhs, rhsList)
         
     def callExpr as Expr
         """

Tests/320-misc-two/810-multi-assignment/300-multiAssign-comma-sep.cobra

@@ +1 @@
+# tests for multiTarget assignment from commasep expression stream
+class MAssign
+    
+    var avar = 99
+    var bvar = 88
+    var _avar1 = 77
+
+    get prop
+        return 66
+    
+    def main is shared
+        ma = MAssign()
+        ma.literals
+        ma.variables
+        ma.exprs
+        ma.asExpected
+        ma.swap
+        
+    def literals    
+        a,b,c = 1,2,3
+        assert a==1
+        assert b==2
+        assert c==3
+    
+        x,y,z = 'this','is','silly'
+        assert x == 'this'
+        assert y == 'is'
+        assert z == 'silly'
+    
+        x,y = 'a','b'
+        assert x == 'a'
+        assert y == 'b'
+        assert z == 'silly'
+        
+        #mixed types
+        a,x,ch = 99,'wokka',c'x'
+        assert a == 99
+        assert x == 'wokka'
+        assert ch == c'x'
+        
+    def variables
+        a = 22
+        b = 47
+        c = 76
+        
+        d,e = a,b
+        assert d == 22
+        assert e == 47
+    
+        d,e = .avar, .bvar
+        assert d == 99
+        assert e == 88
+    
+        d,e = _avar1, 10
+        assert d == 77
+        assert e == 10
+    
+        d,e = -1, _avar1
+        assert d == -1
+        assert e == 77
+        
+        d,c = .prop, a
+        assert d == 66
+        assert c == 22
+        
+    def exprs
+        a,b = 99+1,100-99
+        assert a == 100
+        assert b == 1
+        
+        a = .geti(0)    # OK
+        assert a ==2
+
+        a, = .geti(1),   # OK - note trailing commas
+        assert a == 3
+    
+        #a, = .geti(1)   # typo missing trailing ',': peculiar error: cannot find an indexer
+        #assert a == 3
+        
+        a,b = .geti(2), .geti(10)
+        assert a == 4
+        assert b == 12
+    
+        a,b = 0,0
+        a,b = .geti(2), .geti(10)
+        assert a == 4
+        assert b == 12
+        
+    def geti(i as int) as int
+        return i+2
+
+    def asExpected
+        # test expression values are pinned outside assignment stream
+        a,b = c=2,3
+        assert a == 2 
+        assert c == 2
+        assert b == 3
+        
+        a=100
+        a,b = a+1, a+2
+        assert a == 101
+        assert b == 102
+    
+        c,c = 0,1
+        assert c == 1
+        
+    def swap
+        a = 10
+        b = 20
+        assert a == 10
+        assert b == 20
+        
+        a,b = b,a
+        assert a == 20
+        assert b == 10
+        
+        
+        
+        

Tests/320-misc-two/810-multi-assignment/310-multiAssign-comma-sep-fail.cobra

@@ +1 @@
+# failure cases  for multiTarget assignment from commasep expression stream
+class MAssign
+    def main is shared
+    
+        a,b = 1,2,3 #.error. number of targets (2) must be the same as the number of expressions (3)
+
+        a,b,c = 1,2 #.error. number of targets (3) must be the same as the number of expressions (2)

Developer/IntermediateReleaseNotes.text

@@ -17 +17 @@
 .code
     %% error 'Cannot build this file till mondo-patch installed'
     
-* Added support for "multi-arg assignment" which allows you to assign to a number of variables from contents of a list in one statement
+* Added support for "multi-arg assignment" which allows you to assign to a number of variables from a compatible list of expressions in one statement
 .code
+    a,b,c = 'val1', 999, c'\t'
+    d,e,f = 99+1, 100*1, 101-1
+        a,b = b,a   # one line swap
+or from contents of a list or array (any integer indexable item) in one statement
+.code
     a,b,c = ['val1', 'val2', 'val3']
+    d,e,f = @[99, 100, 101]
 Also support such assignment in for statements for (generic) dictionaries and lists
 .code
     for key, value in aDictionary