Fix compound assignment

src/cmake/testing.cmake

@@ -281,6 +281,7 @@ macro (osl_add_all_tests)
                 color color2 color4 color-reg colorspace comparison
                 complement-reg compile-buffer compassign-bool compassign-reg
                 component-range
+                compound-assignment
                 control-flow-reg connect-components
                 const-array-params const-array-fill
                 debugnan debug-uninit

src/liboslcomp/ast.cpp

@@ -1044,12 +1044,6 @@ ASTassign_expression::ASTassign_expression(OSLCompilerImpl* comp, ASTNode* var,
                                            Operator op, ASTNode* expr)
     : ASTNode(assign_expression_node, comp, op, var, expr)
 {
-    if (op != Assign) {
-        // Rejigger to straight assignment and binary op
-        m_op          = Assign;
-        m_children[1] = new ASTbinary_expression(comp, op, var, expr);
-    }
-
     check_symbol_writeability(var);
 }
 
@@ -1178,6 +1172,24 @@ ASTbinary_expression::ASTbinary_expression(OSLCompilerImpl* comp, Operator op,
 
 
 
+ASTbinary_expression::ASTbinary_expression(OSLCompilerImpl* comp, Operator op,
+                                           ASTNode::ref left,
+                                           ASTNode::ref right)
+    : ASTNode(binary_expression_node, comp, op)
+{
+    addchild(left);
+    addchild(right);
+    // Check for a user-overloaded function for this operator.
+    // Disallow a few ops from overloading.
+    if (op != And && op != Or) {
+        ustring funcname = ustring::fmtformat("__operator__{}__", opword());
+        Symbol* sym      = comp->symtab().find(funcname);
+        if (sym && sym->symtype() == SymTypeFunction)
+            m_function_overload = (FunctionSymbol*)sym;
+    }
+}
+
+
 ASTNode*
 ASTbinary_expression::make(OSLCompilerImpl* comp, Operator op, ASTNode* left,
                            ASTNode* right)

src/liboslcomp/ast.h

@@ -827,6 +827,12 @@ class ASTbinary_expression final : public ASTNode {
     ASTbinary_expression(OSLCompilerImpl* comp, Operator op, ASTNode* left,
                          ASTNode* right);
 
+    // this constructor is used to typecheck the compound assignent (+= and friends)
+    // it's almost identical to the normal one, except that because it takes ref's
+    // it doesn't destroy the children unless it's the right thing to do
+    ASTbinary_expression(OSLCompilerImpl* comp, Operator op, ASTNode::ref left,
+                         ASTNode::ref right);
+
     // Special consructor wrapper that can collapse ops between literals
     static ASTNode* make(OSLCompilerImpl* comp, Operator op, ASTNode* left,
                          ASTNode* right);

src/liboslcomp/codegen.cpp

@@ -465,9 +465,9 @@ ASTcompound_initializer::codegen(Symbol* sym)
 Symbol*
 ASTassign_expression::codegen(Symbol* dest)
 {
-    OSL_DASSERT(m_op == Assign);  // all else handled by binary_op
-
     ASTindex* index = NULL;
+    Symbol *ind = NULL, *ind2 = NULL, *ind3 = NULL;
+
     if (var()->nodetype() == index_node) {
         // Assigning to an individual component or array element
         index = (ASTindex*)var().get();
@@ -482,7 +482,24 @@ ASTassign_expression::codegen(Symbol* dest)
         dest = var()->codegen();
     }
 
-    Symbol* operand = expr()->codegen(dest);
+    // For compound assignments (+=, *=, etc.), read the current LHS value,
+    // apply the binary op, and use the result as the operand for plain assign.
+    // Index expressions are evaluated once and reused for the write because they
+    // are the only ones at the moment that could have side effects.
+    Symbol* operand;
+    if (m_op != Assign) {
+        Symbol* lhs_read;
+        if (index) {
+            lhs_read = index->codegen(NULL, ind, ind2, ind3);
+        } else {
+            lhs_read = var()->codegen();
+        }
+        Symbol* rhs = expr()->codegen();
+        operand     = m_compiler->make_temporary(typespec());
+        emitcode(opword(), operand, lhs_read, rhs);
+    } else {
+        operand = expr()->codegen(dest);
+    }
     OSL_DASSERT(operand != NULL);
 
     if (typespec().is_structure_array()) {
@@ -500,7 +517,8 @@ ASTassign_expression::codegen(Symbol* dest)
         // Assignment of struct copies each element individually
         if (operand != dest) {
             StructSpec* structspec = typespec().structspec();
-            Symbol* arrayindex     = index ? index->index()->codegen() : NULL;
+            Symbol* arrayindex = index ? (ind ? ind : index->index()->codegen())
+                                       : NULL;
             if (arrayindex) {
                 // Special case -- assignment to a element of an array of
                 // structs.  Beware the temp that may have been created above,
@@ -528,7 +546,7 @@ ASTassign_expression::codegen(Symbol* dest)
     }
 
     if (index) {
-        index->codegen_assign(operand);
+        index->codegen_assign(operand, ind, ind2, ind3);
         dest = operand;  // so transitive assignment works for array refs
     } else if (operand != dest) {
         emitcode(typespec().is_array() ? "arraycopy" : "assign", dest, operand);

src/liboslcomp/typecheck.cpp

@@ -300,7 +300,15 @@ ASTassign_expression::typecheck(TypeSpec /*expected*/)
         return TypeSpec();
     }
 
-    OSL_DASSERT(m_op == Assign);  // all else handled by binary_op
+    // For compound assignments (+=, *=, etc. call it @=) we need to
+    // validate that the binary op @ is applicable to var() and expr(),
+    // and then that its result can be assigned into var().
+
+    if (m_op != Assign) {
+        ASTbinary_expression bin(m_compiler, Operator(m_op), var(), expr());
+        et = bin.typecheck(vt);
+    }
+
     ustring varname;
     if (var()->nodetype() == variable_ref_node)
         varname = ((ASTvariable_ref*)var().get())->name();

testsuite/compound-assignment/ref/out.txt

@@ -0,0 +1,7 @@
+Compiled test.osl -> test.oso
+i: 0, j: 0
+i: 1, j: 1
+i: 2, j: 2
+i: 3, j: 3
+a: {2, 4, 6, 8, 0, 0, 0, 0}
+

testsuite/compound-assignment/run.py

@@ -0,0 +1,7 @@
+#!/usr/bin/env python
+
+# Copyright Contributors to the Open Shading Language project.
+# SPDX-License-Identifier: BSD-3-Clause
+# https://github.com/AcademySoftwareFoundation/OpenShadingLanguage
+
+command = testshade("test")

testsuite/compound-assignment/test.osl

@@ -0,0 +1,23 @@
+// Copyright Contributors to the Open Shading Language project.
+// SPDX-License-Identifier: BSD-3-Clause
+// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage
+
+// Test assigning to float from int without temporary works properly.
+// Should really validate the generated oso too.
+
+shader
+test()
+{
+    int a[8] = { 1, 2, 3, 4, 0, 0, 0, 0 };
+    int b[4] = { 1, 2, 3, 4 };
+    int i = 0, j = 0;
+    for (int k = 0; k < 4; ++k) {
+        printf("i: %d, j: %d\n", i, j);
+        a[i++] += b[j++];
+    }
+
+    // Print all elements of 'a' to make sure we didn't write
+    // outside of the scan
+    printf("a: {%d, %d, %d, %d, %d, %d, %d, %d}\n", a[0], a[1], a[2], a[3],
+           a[4], a[5], a[6], a[7]);
+}

testsuite/debug-uninit/ref/out.txt

@@ -5,8 +5,8 @@ ERROR: Detected possible use of uninitialized value in int i_uninit at test.osl:
 ERROR: Detected possible use of uninitialized value in float f_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 5 'color', arg 1)
 ERROR: Detected possible use of uninitialized value in string s_uninit at test.osl:15 (group unnamed_group_1, layer 0 test_0, shader test, op 6 'texture', arg 1)
 ERROR: [RendererServices::texture] ImageInput::create() called with no filename
-ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 12 'aref', arg 1)
-ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 17 'compref', arg 1)
-ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 22 'mxcompref', arg 1)
-ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 49 'spline', arg 4)
-ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 51 'splineinverse', arg 4)
+ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 14 'aref', arg 1)
+ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 21 'compref', arg 1)
+ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 28 'mxcompref', arg 1)
+ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 58 'spline', arg 4)
+ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 61 'splineinverse', arg 4)