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