Closes #889 (plus some cleanups, pong @Shamanas)

source/rock/middle/BinaryOp.ooc

@@ -89,7 +89,13 @@ BinaryOp: class extends Expression {
         new(left clone(), right clone(), type, token)
     }
 
-    isAssign: func -> Bool { (type >= OpType ass) && (type <= OpType bAndAss) }
+    isAssign: func -> Bool {
+        (type >= OpType ass) && (type <= OpType bAndAss)
+    }
+
+    isCompositeAssign: func -> Bool {
+        (type >= OpType addAss) && (type <= OpType bAndAss)
+    }
 
     isBooleanOp: func -> Bool { type == OpType or || type == OpType and }
 
@@ -114,7 +120,7 @@ BinaryOp: class extends Expression {
     }
 
     unwrapAssign: func (trail: Trail, res: Resolver) -> Bool {
-        if(!isAssign()) return false
+        if(!isCompositeAssign()) return false
 
         unwrapGetter := func(e: Expression) -> Expression{
             if(e instanceOf?(VariableAccess) && e as VariableAccess ref instanceOf?(PropertyDecl)) {
@@ -130,7 +136,9 @@ BinaryOp: class extends Expression {
             e
         }
         innerType := type - (OpType addAss - OpType add)
-        inner := BinaryOp new(unwrapGetter(left), unwrapGetter(right), innerType, token)
+        // very important to clone left! otherwise generics won't play well
+        // (behave differently when lhs or rhs, cf. #889)
+        inner := BinaryOp new(unwrapGetter(left clone()), unwrapGetter(right), innerType, token)
         right = inner
         type = OpType ass
 
@@ -455,31 +463,8 @@ BinaryOp: class extends Expression {
             }
         }
 
-        // In case of a expression like `expr attribute += value` where `attribute`
-        // is a property, we need to unwrap this to `expr attribute = expr attribute + value`.
-        if(isAssign() && left instanceOf?(VariableAccess)) {
-            if(left getType() == null || !left isResolved()) {
-                res wholeAgain(this, "left type is unresolved"); return Response OK
-            }
-            if(right getType() == null || !right isResolved()) {
-                res wholeAgain(this, "right type is unresolved"); return Response OK
-            }
-            // are we in a +=, *=, /=, ... operator? unwrap myself.
-            if(left as VariableAccess ref instanceOf?(PropertyDecl)) {
-                leftProperty := left as VariableAccess ref as PropertyDecl
-                if(leftProperty inOuterSpace(trail)) {
-                    // only outside of get/set.
-                    unwrapAssign(trail, res)
-                    trail push(this)
-                    response := right resolve(trail, res)
-                    trail pop(this)
-                    if(!response ok()){
-                        return response
-                    }
-                    return Response LOOP
-                }
-            }
-        }
+        // Do we need to unwrap `a += b` to `a = a + b` ?
+        if (!checkUnwrapAssign(trail, res)) return Response OK
 
         // We must replace the null-coalescing operator with a ternary operator
         if(type == OpType nullCoal) {
@@ -510,6 +495,81 @@ BinaryOp: class extends Expression {
 
     }
 
+    _checkUnwrapAssignDone := false
+
+    /**
+     * Check if we need to unwrap `a += b` into `a = a + b`
+     * @return false if the resolving process for this node needs to
+     * stop for now (ie. after wholeAgain), true if it's all good.
+     */
+    checkUnwrapAssign: func (trail: Trail, res: Resolver) -> Bool {
+
+        if (!isCompositeAssign()) {
+            // well that was quick
+            _checkUnwrapAssignDone = true
+            return true
+        }
+
+        if (!left instanceOf?(VariableAccess)) {
+            // easy one too
+            _checkUnwrapAssignDone = true
+            return true
+        }
+
+        lhsType := left getType()
+        rhsType := right getType()
+
+        if(lhsType == null || lhsType getRef() == null) {
+            res wholeAgain(this, "need left type & its ref")
+            return false
+        }
+
+        if(rhsType == null || rhsType getRef() == null) {
+            res wholeAgain(this, "need right type & its ref")
+            return false
+        }
+
+        lhs := left as VariableAccess
+        lhsRef := lhs getRef()
+
+        match lhsRef {
+            // property assignment (calling setters and getters) only works if we unwrap
+            case lhsPropDecl: PropertyDecl =>
+                if(!lhsPropDecl inOuterSpace(trail)) {
+                    // we're inside a property getter or setter, regular
+                    // rules do not apply (ie. never unwrap)
+                    _checkUnwrapAssignDone = true
+                    return true
+                }
+
+            // generic access / assignment of generic members only works if we unwrap
+            case lhsVarDecl: VariableDecl =>
+                lhsVarDeclType := lhsVarDecl getType()
+                if (lhsVarDeclType == null || lhsVarDeclType getRef() == null) {
+                    res wholeAgain(this, "need lhs var decl type & its ref")
+                    return false
+                }
+
+                if (!lhsVarDeclType isGeneric()) {
+                    // no need to unwrap non-property, non-generic stuff
+                    _checkUnwrapAssignDone = true
+                    return true
+                }
+        }
+
+        // if we reach here, we need to unwrap!
+        unwrapAssign(trail, res)
+
+        // try to avoid looping by resolving right immediately
+        trail push(this)
+        right resolve(trail, res)
+        trail pop(this)
+
+        // now we can go on to the isLegal() test, etc.
+        _checkUnwrapAssignDone = true
+        return true
+    }
+
     isGeneric: func -> Bool {
         (left  getType() isGeneric() && left  getType() pointerLevel() == 0) ||
         (right getType() isGeneric() && right getType() pointerLevel() == 0)

source/rock/middle/VariableAccess.ooc

@@ -3,7 +3,7 @@ import BinaryOp, Visitor, Expression, VariableDecl, FunctionDecl,
        TypeDecl, Declaration, Type, Node, ClassDecl, NamespaceDecl,
        EnumDecl, PropertyDecl, FunctionCall, Module, Import, FuncType,
        NullLiteral, AddressOf, BaseType, StructLiteral, Return,
-       Argument, Scope, CoverDecl, StringLiteral
+       Argument, Scope, CoverDecl, StringLiteral, Cast
 
 import tinker/[Resolver, Response, Trail, Errors]
 import structs/ArrayList
@@ -145,6 +145,8 @@ VariableAccess: class extends Expression {
 
     isResolved: func -> Bool { ref != null && getType() != null && funcTypeDone }
 
+    // TODO: oh boy.. this needs to be broken down into different methods
+
     resolve: func (trail: Trail, res: Resolver) -> Response {
 
         if (isResolved()) {
@@ -195,6 +197,7 @@ VariableAccess: class extends Expression {
                 res wholeAgain(this, "expr type or expr type ref is null")
                 return Response OK
             }
+
             if(!expr getType() getRef() instanceOf?(ClassDecl)) {
                 name = expr getType() getName()
                 ref = expr getType() getRef()
@@ -284,7 +287,7 @@ VariableAccess: class extends Expression {
             }
         }
 
-        if (getType() instanceOf?(FuncType) ) {
+        if (getType() instanceOf?(FuncType)) {
             fType := getType() as FuncType
             parent := trail peek()
 
@@ -327,15 +330,14 @@ VariableAccess: class extends Expression {
                         funcTypeDone = true
                     }
 
-                } else if (parent instanceOf?(BinaryOp)) {
-                    binOp := parent as BinaryOp
-                    if(binOp isAssign() && binOp getRight() == this) {
-                        if(binOp getLeft() getType() == null) {
-                            res wholeAgain(this, "need type of BinOp's lhs")
-                            return Response OK
-                        }
-                        closureType = binOp getLeft() getType() clone()
+                } else if (trail isRHS(this)) {
+                    binOp := trail peek() as BinaryOp
+                    lhsType := binOp left getType()
+                    if(lhsType == null) {
+                        res wholeAgain(this, "need type of BinOp's lhs")
+                        return Response OK
                     }
+                    closureType = lhsType clone()
                 } else if (parent instanceOf?(Return)) {
                     fIndex := trail find(FunctionDecl)
                     if (fIndex != -1) {
@@ -482,10 +484,116 @@ VariableAccess: class extends Expression {
             res wholeAgain(this, "Couldn't resolve varacc")
         }
 
+        checkGenericAccess(trail, res)
+
         return Response OK
 
     }
 
+    _genericAccessDone := false
+
+    checkGenericAccess: func (trail: Trail, res: Resolver) {
+        type := getType()
+        if (!type) {
+            res wholeAgain(this, "need our type")
+            return
+        }
+
+        typeRef := getType() getRef()
+        if (!typeRef) {
+            res wholeAgain(this, "need our type's ref")
+            return
+        }
+
+        if (!type isGeneric()) {
+            // nothing to do for non-generic accesses
+            _genericAccessDone = true
+            return
+        }
+
+        parent := trail peek()
+        match parent {
+            case cast: Cast =>
+                // parent is already an explicit cast, nothing to do
+                _genericAccessDone = true
+                return
+        }
+
+        if (trail isLHS(this)) {
+            // we're being assigned to, no cast needed (nor wanted)
+            _genericAccessDone = true
+            return
+        }
+
+        if (expr == null) {
+            // we can't infer our type without an expr
+            _genericAccessDone = true
+            return
+        }
+
+        exprType := expr getType()
+        if (exprType == null || exprType getRef() == null) {
+            res wholeAgain(expr, "need our expr's ref")
+            return
+        }
+
+        exprTypeRef := exprType getRef()
+        if (debugCondition()) {
+            token printMessage("Found access to generic, type = #{type}, expr = #{expr}, expr type = #{exprType}, expr type ref = #{exprTypeRef}", "")
+        }
+        exprTypeDecl: TypeDecl
+        match exprTypeRef {
+            case td: TypeDecl =>
+                exprTypeDecl = td
+            case =>
+                msg := "Expected #{expr}'s typeRef to be a TypeDecl"
+                err := InternalError new(token, msg)
+                res throwError(err)
+        }
+
+        lhsTypeArgs := exprTypeDecl getTypeArgs()
+        rhsTypeArgs := exprType getTypeArgs()
+
+        if (rhsTypeArgs == null) {
+            // not in a qualified expr, can't infer
+            _genericAccessDone = true
+            return
+        }
+
+        if (lhsTypeArgs size != rhsTypeArgs size) {
+            msg := "Inconsistent typeArgs count between #{type} and #{exprType} in generic variable access"
+            err := InternalError new(token, msg)
+            res throwError(err)
+            return
+        }
+
+        ourTypeArg := type getName()
+        for (i in 0..lhsTypeArgs size) {
+            lhsType := lhsTypeArgs get(i)
+            rhsType := rhsTypeArgs get(i)
+
+            if (lhsType name == ourTypeArg) {
+                if (rhsType isGeneric()) {
+                    // that's no real type at all, we're probably in that same
+                    // generic class definition
+
+                    // TODO: need a better check, maybe that rhsType's ref is
+                    // a VariableDecl?
+                    _genericAccessDone = true
+                    return
+                }
+
+                cast := Cast new(this, rhsType inner clone(), token)
+                if (!parent replace(this, cast)) {
+                    res throwError(CouldntReplace new(token, this, cast, trail))
+                }
+                res wholeAgain(this, "Just realtypized ourselves")
+                _genericAccessDone = true
+                return
+            }
+        }
+    }
+
     findSimilar: func (res: Resolver) -> String {
 
         buff := Buffer new()

source/rock/middle/tinker/Trail.ooc

@@ -1,5 +1,5 @@
 import structs/[Stack, ArrayList]
-import ../[Node, Module, Statement, Scope, If, Else]
+import ../[Node, Module, Statement, Scope, If, Else, BinaryOp]
 
 Trail: class extends Stack<Node> {
 
@@ -19,7 +19,7 @@ Trail: class extends Stack<Node> {
      */
     pop: func ~verify (reference: Node) -> Node {
 
-        popped : Node = pop()
+        popped: Node = pop()
         if(popped != reference) {
             Exception new(This, "Should have popped " +
                 reference toString() + " but popped " + popped toString()) throw()
@@ -172,6 +172,32 @@ Trail: class extends Stack<Node> {
 
     }
 
+    /**
+     * @return true when `node`'s parent is a BinaryOp, of one of the
+     * assignment types, and `node` is on the left-hand side (ie. being
+     * assigned to)
+     */
+    isLHS: func (node: Node) -> Bool {
+        match (parent := peek()) {
+            case bop: BinaryOp =>
+                return bop isAssign() && bop left == node
+        }
+        false
+    }
+
+    /**
+     * @return true when `node`'s parent is a BinaryOp, of one of the
+     * assignment types, and `node` is on the right-hand side (ie. being
+     * assigned)
+     */
+    isRHS: func (node: Node) -> Bool {
+        match (parent := peek()) {
+            case bop: BinaryOp =>
+                return bop isAssign() && bop right == node
+        }
+        false
+    }
+
     /**
      * Delegate for data get(index)
      */

test/compiler/generics/generic-member.ooc

@@ -0,0 +1,14 @@
+
+// Test for https://github.com/fasterthanlime/rock/issues/889
+
+use sam-assert
+
+describe("accessing generic member should not require a cast", ||
+    g := Gift<String> new("hi")
+    g tuvalu println()
+)
+
+Gift: class <T> {
+    tuvalu: T
+    init: func (=tuvalu)
+}