From 0c2bb7111825c706105b85807526c11eeebaef9b Mon Sep 17 00:00:00 2001 From: Paul King Date: Tue, 28 Apr 2026 22:08:01 +1000 Subject: [PATCH] GROOVY-11970: Provide support for compound assignment operator overloading (GEP-15) --- .../java/groovy/transform/OperatorRename.java | 24 ++ .../codehaus/groovy/classgen/Verifier.java | 7 + .../classgen/asm/BinaryExpressionHelper.java | 69 +++- .../BinaryExpressionMultiTypeDispatcher.java | 9 + ...esBinaryExpressionMultiTypeDispatcher.java | 48 ++- .../groovy/runtime/ScriptBytecodeAdapter.java | 20 ++ .../org/codehaus/groovy/syntax/Types.java | 3 +- .../OperatorRenameASTTransformation.java | 63 +++- .../stc/StaticTypeCheckingSupport.java | 25 ++ .../stc/StaticTypeCheckingVisitor.java | 42 +++ .../transform/stc/StaticTypesMarker.java | 4 +- src/spec/doc/core-operators.adoc | 38 +++ .../operator/CompoundAssignmentTest.groovy | 321 ++++++++++++++++++ 13 files changed, 655 insertions(+), 18 deletions(-) create mode 100644 src/test/groovy/groovy/operator/CompoundAssignmentTest.groovy diff --git a/src/main/java/groovy/transform/OperatorRename.java b/src/main/java/groovy/transform/OperatorRename.java index 97fdb0ee7c0..995cb1c310c 100644 --- a/src/main/java/groovy/transform/OperatorRename.java +++ b/src/main/java/groovy/transform/OperatorRename.java @@ -152,4 +152,28 @@ * @return the replacement method name */ String compareTo() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code +=}. */ + String plusAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code -=}. */ + String minusAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code *=}. */ + String multiplyAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code /=}. */ + String divAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code %=}. */ + String remainderAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code **=}. */ + String powerAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code <<=}. */ + String leftShiftAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code >>=}. */ + String rightShiftAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code >>>=}. */ + String rightShiftUnsignedAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code &=}. */ + String andAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code |=}. */ + String orAssign() default Undefined.STRING; + /** GEP-15: rename the dedicated compound-assignment method for {@code ^=}. */ + String xorAssign() default Undefined.STRING; } diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index e9fdb4c782b..108b0fb8261 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -126,6 +126,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType; import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod; import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.STATIC_COMPILE_NODE; +import static org.codehaus.groovy.transform.stc.StaticTypesMarker.COMPOUND_ASSIGN_TARGET; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET; /** @@ -349,6 +350,12 @@ public void variableNotFinal(Variable var, final Expression bexp) { if (var instanceof VariableExpression) { var = ((VariableExpression) var).getAccessedVariable(); } + // GEP-15: a compound-assign that resolved to a *Assign method (e.g. plusAssign) + // mutates the receiver in place rather than reassigning the variable, so the + // final-reassignment check must not fire for it. + if (bexp instanceof BinaryExpression be && be.getNodeMetaData(COMPOUND_ASSIGN_TARGET) != null) { + return; + } if (var instanceof VariableExpression && isFinal(var.getModifiers())) { throw new RuntimeParserException("The variable [" + var.getName() + "] is declared final but is reassigned", bexp); } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java index a2d83e8a37e..b39bdf208d4 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java @@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.MultipleAssignmentMetadata; import org.codehaus.groovy.ast.Variable; +import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.ArrayExpression; import org.codehaus.groovy.ast.expr.BinaryExpression; import org.codehaus.groovy.ast.expr.ClassExpression; @@ -36,6 +37,7 @@ import org.codehaus.groovy.ast.expr.PostfixExpression; import org.codehaus.groovy.ast.expr.PrefixExpression; import org.codehaus.groovy.ast.expr.PropertyExpression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; import org.codehaus.groovy.ast.expr.TernaryExpression; import org.codehaus.groovy.ast.expr.TupleExpression; import org.codehaus.groovy.ast.expr.VariableExpression; @@ -209,7 +211,7 @@ public void eval(final BinaryExpression expression) { break; case BITWISE_AND_EQUAL: - evaluateBinaryExpressionWithAssignment("and", expression); + evaluateCompoundAssign("andAssign", "and", expression); break; case BITWISE_OR: @@ -217,7 +219,7 @@ public void eval(final BinaryExpression expression) { break; case BITWISE_OR_EQUAL: - evaluateBinaryExpressionWithAssignment("or", expression); + evaluateCompoundAssign("orAssign", "or", expression); break; case BITWISE_XOR: @@ -229,7 +231,7 @@ public void eval(final BinaryExpression expression) { break; case BITWISE_XOR_EQUAL: - evaluateBinaryExpressionWithAssignment("xor", expression); + evaluateCompoundAssign("xorAssign", "xor", expression); break; case PLUS: @@ -237,7 +239,7 @@ public void eval(final BinaryExpression expression) { break; case PLUS_EQUAL: - evaluateBinaryExpressionWithAssignment("plus", expression); + evaluateCompoundAssign("plusAssign", "plus", expression); break; case MINUS: @@ -245,7 +247,7 @@ public void eval(final BinaryExpression expression) { break; case MINUS_EQUAL: - evaluateBinaryExpressionWithAssignment("minus", expression); + evaluateCompoundAssign("minusAssign", "minus", expression); break; case MULTIPLY: @@ -253,7 +255,7 @@ public void eval(final BinaryExpression expression) { break; case MULTIPLY_EQUAL: - evaluateBinaryExpressionWithAssignment("multiply", expression); + evaluateCompoundAssign("multiplyAssign", "multiply", expression); break; case DIVIDE: @@ -263,7 +265,7 @@ public void eval(final BinaryExpression expression) { case DIVIDE_EQUAL: //SPG don't use divide since BigInteger implements directly //and we want to dispatch through DefaultGroovyMethods to get a BigDecimal result - evaluateBinaryExpressionWithAssignment("div", expression); + evaluateCompoundAssign("divAssign", "div", expression); break; case INTDIV: @@ -271,6 +273,7 @@ public void eval(final BinaryExpression expression) { break; case INTDIV_EQUAL: + // GEP-15 explicitly excludes \= (no intdivAssign convention) evaluateBinaryExpressionWithAssignment("intdiv", expression); break; @@ -279,7 +282,9 @@ public void eval(final BinaryExpression expression) { break; case MOD_EQUAL: - evaluateBinaryExpressionWithAssignment("mod", expression); + // GEP-15 maps both MOD_EQUAL and REMAINDER_EQUAL to remainderAssign for consistency + // with getOperationName collapse, even though current parser only emits REMAINDER_EQUAL. + evaluateCompoundAssign("remainderAssign", "mod", expression); break; case REMAINDER: @@ -287,7 +292,7 @@ public void eval(final BinaryExpression expression) { break; case REMAINDER_EQUAL: - evaluateBinaryExpressionWithAssignment("remainder", expression); + evaluateCompoundAssign("remainderAssign", "remainder", expression); break; case POWER: @@ -295,7 +300,7 @@ public void eval(final BinaryExpression expression) { break; case POWER_EQUAL: - evaluateBinaryExpressionWithAssignment("power", expression); + evaluateCompoundAssign("powerAssign", "power", expression); break; case ELVIS_EQUAL: @@ -307,7 +312,7 @@ public void eval(final BinaryExpression expression) { break; case LEFT_SHIFT_EQUAL: - evaluateBinaryExpressionWithAssignment("leftShift", expression); + evaluateCompoundAssign("leftShiftAssign", "leftShift", expression); break; case RIGHT_SHIFT: @@ -315,7 +320,7 @@ public void eval(final BinaryExpression expression) { break; case RIGHT_SHIFT_EQUAL: - evaluateBinaryExpressionWithAssignment("rightShift", expression); + evaluateCompoundAssign("rightShiftAssign", "rightShift", expression); break; case RIGHT_SHIFT_UNSIGNED: @@ -323,7 +328,7 @@ public void eval(final BinaryExpression expression) { break; case RIGHT_SHIFT_UNSIGNED_EQUAL: - evaluateBinaryExpressionWithAssignment("rightShiftUnsigned", expression); + evaluateCompoundAssign("rightShiftUnsignedAssign", "rightShiftUnsigned", expression); break; case KEYWORD_INSTANCEOF: @@ -891,6 +896,44 @@ protected void evaluateBinaryExpressionWithAssignment(final String method, final controller.getCompileStack().popLHS(); } + /** + * GEP-15: dynamic-mode compound-assign codegen. Routes through + * {@link ScriptBytecodeAdapter#compoundAssign(Object, Object, String, String)} + * which dispatches to {@code assignName} when the receiver responds to it, + * and falls back to {@code baseName} otherwise. The caller stores the helper's + * return value into the LHS — for the in-place branch this is a no-op store + * of the receiver back to itself; for the fallback branch it is the usual + * "x = x.op(y)" assignment. + */ + protected void evaluateCompoundAssign(final String assignName, final String baseName, final BinaryExpression expression) { + Expression leftExpression = expression.getLeftExpression(); + if (leftExpression instanceof BinaryExpression bexp + && bexp.getOperation().getType() == LEFT_SQUARE_BRACKET) { + // Subscript LHS (e.g. a[i] += b) is intentionally out of scope for GEP-15; + // keep the legacy getAt/putAt-based path. + evaluateArrayAssignmentWithOperator(baseName, expression, bexp); + return; + } + + StaticMethodCallExpression helperCall = new StaticMethodCallExpression( + ClassHelper.make(ScriptBytecodeAdapter.class), + "compoundAssign", + new ArgumentListExpression(new Expression[]{ + leftExpression, + expression.getRightExpression(), + new ConstantExpression(assignName), + new ConstantExpression(baseName) + }) + ); + helperCall.setSourcePosition(expression); + helperCall.visit(controller.getAcg()); + + controller.getOperandStack().dup(); + controller.getCompileStack().pushLHS(true); + leftExpression.visit(controller.getAcg()); + controller.getCompileStack().popLHS(); + } + private void evaluateInstanceof(final BinaryExpression expression) { CompileStack compileStack = controller.getCompileStack(); OperandStack operandStack = controller.getOperandStack(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java index 9a98f459be6..929f44569aa 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java @@ -332,6 +332,15 @@ protected void evaluateBinaryExpressionWithAssignment(final String method, final super.evaluateBinaryExpressionWithAssignment(method, binExp); } + @Override + protected void evaluateCompoundAssign(final String assignName, final String baseName, final BinaryExpression binExp) { + // GEP-15: keep the primitive-int/long/float/double fast paths for int i += j etc.; + // they cannot have a *Assign override anyway since primitives are excluded. + if (doAssignmentToArray(binExp)) return; + if (doAssignmentToLocalVariable(baseName, binExp)) return; + super.evaluateCompoundAssign(assignName, baseName, binExp); + } + private boolean doAssignmentToLocalVariable(final String method, final BinaryExpression binExp) { Expression left = binExp.getLeftExpression(); if (left instanceof VariableExpression ve) { diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java index eeee08d167a..1c57e84290e 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java @@ -67,6 +67,7 @@ import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.ARRAYLIST_CONSTRUCTOR; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isAssignment; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.inferLoopElementType; +import static org.codehaus.groovy.transform.stc.StaticTypesMarker.COMPOUND_ASSIGN_TARGET; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.INFERRED_TYPE; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; @@ -136,6 +137,28 @@ private static void visitInsnByType(final ClassNode top, final MethodVisitor mv, @Override protected void evaluateBinaryExpressionWithAssignment(final String method, final BinaryExpression expression) { + if (tryStaticCompoundAssignPaths(method, expression)) return; + super.evaluateBinaryExpressionWithAssignment(method, expression); + } + + @Override + protected void evaluateCompoundAssign(final String assignName, final String baseName, final BinaryExpression expression) { + if (tryStaticCompoundAssignPaths(baseName, expression)) return; + super.evaluateCompoundAssign(assignName, baseName, expression); + } + + /** + * GEP-15 + legacy setter fast-path. Returns true when codegen has been emitted + * (no further dispatch required). + */ + private boolean tryStaticCompoundAssignPaths(final String baseName, final BinaryExpression expression) { + MethodNode assignTarget = expression.getNodeMetaData(COMPOUND_ASSIGN_TARGET); + if (assignTarget != null) { + // GEP-15: receiver.(arg); receiver remains the expression value. + // The setter (for property LHS) is intentionally skipped. + emitCompoundAssignCall(assignTarget, expression); + return true; + } Expression leftExpression = expression.getLeftExpression(); if (leftExpression instanceof PropertyExpression pexp && !(leftExpression instanceof AttributeExpression)) { @@ -161,10 +184,31 @@ protected void evaluateBinaryExpressionWithAssignment(final String method, final pexp.isSpreadSafe(), pexp.isImplicitThis(), true)) { // TODO: GROOVY-11843 - return; + return true; } } - super.evaluateBinaryExpressionWithAssignment(method, expression); + return false; + } + + private void emitCompoundAssignCall(final MethodNode target, final BinaryExpression expression) { + OperandStack operandStack = controller.getOperandStack(); + Expression leftExpression = expression.getLeftExpression(); + Expression rightExpression = expression.getRightExpression(); + + leftExpression.visit(controller.getAcg()); + ClassNode receiverType = operandStack.getTopOperand(); + int slot = controller.getCompileStack().defineTemporaryVariable("$gep15recv", receiverType, true); + + VariableSlotLoader callReceiver = new VariableSlotLoader(receiverType, slot, operandStack); + MethodCallExpression call = callX(callReceiver, target.getName(), rightExpression); + call.setMethodTarget(target); + call.setImplicitThis(false); + call.setSourcePosition(expression); + call.visit(controller.getAcg()); + operandStack.pop(); // discard the *Assign return value + + new VariableSlotLoader(receiverType, slot, operandStack).visit(controller.getAcg()); + controller.getCompileStack().removeVar(slot); } @Override diff --git a/src/main/java/org/codehaus/groovy/runtime/ScriptBytecodeAdapter.java b/src/main/java/org/codehaus/groovy/runtime/ScriptBytecodeAdapter.java index 8f4cde24216..8e54adacfa0 100644 --- a/src/main/java/org/codehaus/groovy/runtime/ScriptBytecodeAdapter.java +++ b/src/main/java/org/codehaus/groovy/runtime/ScriptBytecodeAdapter.java @@ -998,4 +998,24 @@ public static Object bitwiseNegate(final Object value) throws Throwable { throw unwrap(gre); } } + + /** + * GEP-15: dispatcher for compound-assignment operators in dynamic Groovy. + * If {@code receiver} responds to {@code assignName} with the supplied argument, + * invoke it and return {@code receiver} (so the caller's STORE assigns the same + * reference back, leaving the in-place mutation visible). Otherwise fall back to + * {@code baseName} and return its result for the caller to assign. + */ + public static Object compoundAssign(final Object receiver, final Object arg, + final String assignName, final String baseName) throws Throwable { + if (receiver != null) { + MetaClass mc = InvokerHelper.getMetaClass(receiver); + if (!mc.respondsTo(receiver, assignName, new Object[]{arg}).isEmpty()) { + invokeMethodN(ScriptBytecodeAdapter.class, receiver, assignName, new Object[]{arg}); + return receiver; + } + } + return invokeMethodN(ScriptBytecodeAdapter.class, receiver, baseName, new Object[]{arg}); + } + } diff --git a/src/main/java/org/codehaus/groovy/syntax/Types.java b/src/main/java/org/codehaus/groovy/syntax/Types.java index 12597e37b48..dca0a9a27ac 100644 --- a/src/main/java/org/codehaus/groovy/syntax/Types.java +++ b/src/main/java/org/codehaus/groovy/syntax/Types.java @@ -415,7 +415,8 @@ public static boolean ofType(int specific, int general) { case ASSIGNMENT_OPERATOR: return specific == EQUAL || (specific >= PLUS_EQUAL && specific <= ELVIS_EQUAL) || (specific >= LOGICAL_OR_EQUAL && specific <= LOGICAL_AND_EQUAL) || (specific >= LEFT_SHIFT_EQUAL && specific <= RIGHT_SHIFT_UNSIGNED_EQUAL) - || (specific >= BITWISE_OR_EQUAL && specific <= BITWISE_XOR_EQUAL); + || (specific >= BITWISE_OR_EQUAL && specific <= BITWISE_XOR_EQUAL) + || specific == REMAINDER_EQUAL; case COMPARISON_OPERATOR: return specific >= COMPARE_NOT_EQUAL && specific <= COMPARE_TO; diff --git a/src/main/java/org/codehaus/groovy/transform/OperatorRenameASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/OperatorRenameASTTransformation.java index 03ade2e2b5b..3ce7be905ef 100644 --- a/src/main/java/org/codehaus/groovy/transform/OperatorRenameASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/OperatorRenameASTTransformation.java @@ -27,11 +27,15 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.BinaryExpression; import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; +import org.codehaus.groovy.runtime.ScriptBytecodeAdapter; import org.objectweb.asm.Opcodes; import java.util.Arrays; @@ -80,6 +84,7 @@ public class OperatorRenameASTTransformation extends ClassCodeExpressionTransfor private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private SourceUnit sourceUnit; private Map nameTable = new HashMap<>(); + private Map assignNameTable = new HashMap<>(); @Override public void visit(ASTNode[] nodes, SourceUnit source) { @@ -103,6 +108,20 @@ public void visit(ASTNode[] nodes, SourceUnit source) { addIfFound(anno, nameTable, "or"); addIfFound(anno, nameTable, "xor"); addIfFound(anno, nameTable, "compareTo"); + // GEP-15: dedicated compound-assignment renames; when present, take precedence over + // the base-method rename for the matching compound-assign operator. + addIfFound(anno, assignNameTable, "plusAssign"); + addIfFound(anno, assignNameTable, "minusAssign"); + addIfFound(anno, assignNameTable, "multiplyAssign"); + addIfFound(anno, assignNameTable, "divAssign"); + addIfFound(anno, assignNameTable, "remainderAssign"); + addIfFound(anno, assignNameTable, "powerAssign"); + addIfFound(anno, assignNameTable, "leftShiftAssign"); + addIfFound(anno, assignNameTable, "rightShiftAssign"); + addIfFound(anno, assignNameTable, "rightShiftUnsignedAssign"); + addIfFound(anno, assignNameTable, "andAssign"); + addIfFound(anno, assignNameTable, "orAssign"); + addIfFound(anno, assignNameTable, "xorAssign"); if (parent instanceof ClassNode) { super.visitClass((ClassNode) parent); } else if (parent instanceof ConstructorNode) { @@ -122,9 +141,33 @@ public Expression transform(Expression expr) { if (expr == null) return null; if (expr instanceof BinaryExpression be) { int type = be.getOperation().getType(); + boolean isEqualOperator = removeAssignment(type) != type; + // GEP-15: a *Assign rename takes precedence over the base rename for compound-assign tokens. + // Routed through ScriptBytecodeAdapter.compoundAssign with the renamed method as both the + // assign and fallback name, so the expression value of `x op= y` is the (mutated) receiver + // `x`, matching the contract for non-renamed *Assign overloads, rather than the (possible + // void/null) return of the renamed method. + if (isEqualOperator) { + String assignOldName = getAssignOperationName(type); + if (assignOldName != null && assignNameTable.containsKey(assignOldName)) { + Expression left = transform(be.getLeftExpression()); + Expression right = transform(be.getRightExpression()); + ConstantExpression renamed = new ConstantExpression(assignNameTable.get(assignOldName)); + Expression result = new StaticMethodCallExpression( + make(ScriptBytecodeAdapter.class), + "compoundAssign", + new ArgumentListExpression(new Expression[]{ + left, + right, + renamed, + renamed + })); + result.setSourcePosition(be); + return result; + } + } String oldName = getOperationName(type); if (nameTable.containsKey(oldName)) { - boolean isEqualOperator = removeAssignment(type) != type; Expression left = transform(be.getLeftExpression()); Expression right = transform(be.getRightExpression()); Expression result = callX(left, nameTable.get(oldName), right); @@ -140,6 +183,24 @@ public Expression transform(Expression expr) { return expr.transformExpression(this); } + static String getAssignOperationName(final int op) { + return switch (op) { + case PLUS_EQUAL -> "plusAssign"; + case MINUS_EQUAL -> "minusAssign"; + case MULTIPLY_EQUAL -> "multiplyAssign"; + case DIVIDE_EQUAL -> "divAssign"; + case REMAINDER_EQUAL -> "remainderAssign"; + case POWER_EQUAL -> "powerAssign"; + case LEFT_SHIFT_EQUAL -> "leftShiftAssign"; + case RIGHT_SHIFT_EQUAL -> "rightShiftAssign"; + case RIGHT_SHIFT_UNSIGNED_EQUAL -> "rightShiftUnsignedAssign"; + case BITWISE_AND_EQUAL -> "andAssign"; + case BITWISE_OR_EQUAL -> "orAssign"; + case BITWISE_XOR_EQUAL -> "xorAssign"; + default -> null; + }; + } + @Override protected SourceUnit getSourceUnit() { return sourceUnit; diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java index b397643b694..e644db409c8 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java @@ -526,6 +526,31 @@ static String getOperationName(final int op) { }; } + /** + * GEP-15: returns the dedicated compound-assignment method name for a compound-assign + * operator token (e.g. {@code PLUS_EQUAL} -> {@code "plusAssign"}), or {@code null} if + * the token is not one of the twelve operators in scope. {@code INTDIV_EQUAL}, + * {@code MOD_EQUAL}, {@code ELVIS_EQUAL}, {@code LOGICAL_OR_EQUAL} and + * {@code LOGICAL_AND_EQUAL} are intentionally excluded. + */ + public static String getAssignOperationName(final int op) { + return switch (op) { + case PLUS_EQUAL -> "plusAssign"; + case MINUS_EQUAL -> "minusAssign"; + case MULTIPLY_EQUAL -> "multiplyAssign"; + case DIVIDE_EQUAL -> "divAssign"; + case REMAINDER_EQUAL -> "remainderAssign"; + case POWER_EQUAL -> "powerAssign"; + case LEFT_SHIFT_EQUAL -> "leftShiftAssign"; + case RIGHT_SHIFT_EQUAL -> "rightShiftAssign"; + case RIGHT_SHIFT_UNSIGNED_EQUAL -> "rightShiftUnsignedAssign"; + case BITWISE_AND_EQUAL -> "andAssign"; + case BITWISE_OR_EQUAL -> "orAssign"; + case BITWISE_XOR_EQUAL -> "xorAssign"; + default -> null; + }; + } + static boolean isShiftOperation(final String name) { return "leftShift".equals(name) || "rightShift".equals(name) || "rightShiftUnsigned".equals(name); } diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index f949d32f2b3..26b807756ad 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -296,6 +296,7 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findTargetVariable; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.fullyResolve; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.fullyResolveType; +import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getAssignOperationName; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getCombinedBoundType; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getCombinedGenericsType; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.getGenericsWithoutArray; @@ -326,6 +327,7 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.typeCheckMethodArgumentWithGenerics; import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.typeCheckMethodsWithGenerics; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.CLOSURE_ARGUMENTS; +import static org.codehaus.groovy.transform.stc.StaticTypesMarker.COMPOUND_ASSIGN_TARGET; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DECLARATION_INFERRED_TYPE; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DELEGATION_METADATA; import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET; @@ -828,6 +830,46 @@ public void visitBinaryExpression(final BinaryExpression expression) { Expression rightExpression = expression.getRightExpression(); if (isAssignment(op) && op != EQUAL) { + // GEP-15: try a dedicated *Assign method on the LHS type before the legacy desugar. + // When found, the receiver is mutated in place — no reassignment, setter is skipped. + String assignName = getAssignOperationName(op); + if (assignName != null) { + // Visit a clone of the LHS so that the property/variable visitor doesn't + // treat it as the assignment target (which would steer it to the setter and + // leave the type unresolved on the read side). + ExpressionTransformer readSideCloner = new ExpressionTransformer() { + @Override + public Expression transform(final Expression expr) { + if (expr instanceof VariableExpression) { + return ((VariableExpression) expr).clone(); + } + return expr.transformExpression(this); + } + }; + Expression lhsRead = readSideCloner.transform(leftExpression); + lhsRead.visit(this); + ClassNode lhsType = getType(lhsRead); + if (!isPrimitiveType(lhsType)) { + rightExpression.visit(this); + ClassNode rhsType = isNullConstant(rightExpression) + ? UNKNOWN_PARAMETER_TYPE + : getInferredTypeFromTempInfo(rightExpression, getType(rightExpression)); + List candidates = findMethod(lhsType, assignName, rhsType); + if (candidates.size() == 1) { + MethodNode target = candidates.get(0); + expression.putNodeMetaData(COMPOUND_ASSIGN_TARGET, target); + storeType(expression, lhsType); + // Visit the actual LHS now (without the cloned read-side context) so its + // own metadata is populated for codegen, but skip its setterInfo since + // *Assign mutates the receiver in place rather than going through a setter. + leftExpression.visit(this); + removeSetterInfo(leftExpression); + typeCheckMethodsWithGenericsOrFail(lhsType, new ClassNode[]{rhsType}, target, rightExpression); + return; + } + } + } + ExpressionTransformer cloneMaker = new ExpressionTransformer() { @Override public Expression transform(final Expression expr) { diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypesMarker.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypesMarker.java index cd8e9ddff2c..028b97b1ea7 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypesMarker.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypesMarker.java @@ -61,5 +61,7 @@ public enum StaticTypesMarker { /** indicates a parameter or method return is known to be non-null (e.g., inferred from {@code @Requires}/{@code @Ensures} contracts) */ INFERRED_NON_NULL, /** list of {@code return null} statements recorded on a method before its body is rewritten, so a downstream checker can still report them as non-null violations */ - INFERRED_NON_NULL_RETURN_VIOLATIONS + INFERRED_NON_NULL_RETURN_VIOLATIONS, + /** GEP-15: stores the resolved compound-assignment {@code MethodNode} (e.g. {@code plusAssign}) on a {@code BinaryExpression} when the static type checker has located one, signalling to codegen that the receiver should be mutated in place rather than {@code x = x.plus(y)}-desugared */ + COMPOUND_ASSIGN_TARGET } diff --git a/src/spec/doc/core-operators.adoc b/src/spec/doc/core-operators.adoc index f195e07c8c5..9daa5a09a17 100644 --- a/src/spec/doc/core-operators.adoc +++ b/src/spec/doc/core-operators.adoc @@ -1007,3 +1007,41 @@ Here is a complete list of the operators and their corresponding methods: | `~a` | a.bitwiseNegate() |==== + +[[Compound-Assignment-Overloading]] +=== Compound assignment operator overloading + +Compound assignment operators (`+=`, `-=`, `\*=`, ...) normally desugar to `x = x.op(y)`, +which creates a new value and reassigns the variable. Since Groovy 5.0 (GEP-15), classes can +define dedicated *Assign methods to override compound-assign independently of the base operator. +When a `*Assign` method exists on the receiver type and the compiler can resolve it directly +(for example, under `@CompileStatic` or `@TypeChecked`), it is called directly: the receiver is +mutated in place, no reassignment occurs (so `final` fields and variables can use compound +assignment in that case), and for property LHS the setter is *not* invoked. In purely dynamic +code the dispatch happens at runtime; the in-place behaviour still applies but the +`final`-reassignment check cannot be relaxed because the compiler does not yet know whether +a `*Assign` method will respond at runtime. + +If no `*Assign` method matches, the legacy `x = x.op(y)` desugar still applies. + +[cols="1,2,2" options="header"] +|==== +| Operator | Assign method | Fallback method + +| `+=` | `plusAssign` | `plus` +| `-=` | `minusAssign` | `minus` +| `*=` | `multiplyAssign` | `multiply` +| `/=` | `divAssign` | `div` +| `%=` | `remainderAssign` | `remainder` +| `\**=` | `powerAssign` | `power` +| `<<=` | `leftShiftAssign` | `leftShift` +| `>>=` | `rightShiftAssign` | `rightShift` +| `>>>=` | `rightShiftUnsignedAssign` | `rightShiftUnsigned` +| `&=` | `andAssign` | `and` +| `\|=` | `orAssign` | `or` +| `^=` | `xorAssign` | `xor` +|==== + +When `*Assign` is called, the expression value of `x op= y` is the (mutated) `x`, not the +return value of `*Assign`. `*Assign` is also discoverable as an extension method or category, +and its name can be remapped via `@OperatorRename(plusAssign="addInPlace")`. diff --git a/src/test/groovy/groovy/operator/CompoundAssignmentTest.groovy b/src/test/groovy/groovy/operator/CompoundAssignmentTest.groovy new file mode 100644 index 00000000000..fcf2f707401 --- /dev/null +++ b/src/test/groovy/groovy/operator/CompoundAssignmentTest.groovy @@ -0,0 +1,321 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package groovy.operator + +import groovy.test.GroovyTestCase + +/** + * GEP-15: tests for dedicated compound-assignment operator overloads + * (plusAssign, minusAssign, ...). + * + * Test scripts use a uniquely-named entry method (not run()) so they do not + * collide with the script class's auto-generated run() method. + */ +final class CompoundAssignmentTest extends GroovyTestCase { + + void testStaticPlusAssignChosenOverPlus() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Acc { + int total = 0 + int plusCalls = 0 + int plusAssignCalls = 0 + void plusAssign(int n) { total += n; plusAssignCalls++ } + Acc plus(int n) { plusCalls++; def a = new Acc(); a.total = total + n; a } + } + + @CompileStatic + void exercise() { + def a = new Acc() + a += 5 + assert a.total == 5 + assert a.plusAssignCalls == 1 + assert a.plusCalls == 0 + def b = a + 10 + assert a.plusCalls == 1 + assert b.total == 15 + } + exercise() + ''' + } + + void testStaticPlusFallbackWhenNoPlusAssign() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class V { + int n = 0 + V plus(int x) { def v = new V(); v.n = n + x; v } + } + + @CompileStatic + void exercise() { + def v = new V() + v += 7 + assert v.n == 7 + } + exercise() + ''' + } + + void testStaticExpressionValueIsReceiver() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class C { + int n = 0 + void plusAssign(int x) { n += x } + } + + @CompileStatic + void exercise() { + def c = new C() + def r = (c += 4) + assert c.n == 4 + assert r.is(c) + } + exercise() + ''' + } + + void testDynamicPlusAssignChosenOverPlus() { + assertScript ''' + class Acc { + int total = 0 + int plusCalls = 0 + int plusAssignCalls = 0 + void plusAssign(int n) { total += n; plusAssignCalls++ } + Acc plus(int n) { plusCalls++; def a = new Acc(); a.total = total + n; a } + } + + def a = new Acc() + a += 5 + assert a.total == 5 + assert a.plusAssignCalls == 1 + assert a.plusCalls == 0 + ''' + } + + void testDynamicPlusFallbackWhenNoPlusAssign() { + assertScript ''' + class V { + int n = 0 + V plus(int x) { def v = new V(); v.n = n + x; v } + } + + def v = new V() + v += 7 + assert v.n == 7 + ''' + } + + void testStaticPropertyCompoundAssignSkipsSetter() { + assertScript ''' + import groovy.transform.CompileStatic + + class Counter { + int n = 0 + void plusAssign(int x) { n += x } + } + + @CompileStatic + class Holder { + Counter counter = new Counter() + int setCounterCalls = 0 + void setCounter(Counter c) { setCounterCalls++; this.@counter = c } + } + + @CompileStatic + void exercise() { + def h = new Holder() + h.counter += 3 + assert h.counter.n == 3 + assert h.setCounterCalls == 0 + } + exercise() + ''' + } + + void testStaticPrimitiveFastPathUnaffected() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + int sum(int n) { + int total = 0 + for (i in 0..n) total += i + total + } + + assert sum(10) == 55 + ''' + } + + void testStaticFinalLocalWithPlusAssign() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Counter { + int n = 0 + void plusAssign(int x) { n += x } + } + + @CompileStatic + void exercise() { + final Counter c = new Counter() + c += 5 + assert c.n == 5 + } + exercise() + ''' + } + + void testStaticFinalFieldWithPlusAssign() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Counter { + int n = 0 + void plusAssign(int x) { n += x } + } + + @CompileStatic + class Holder { + final Counter c = new Counter() + } + + @CompileStatic + void exercise() { + def h = new Holder() + h.c += 5 + assert h.c.n == 5 + } + exercise() + ''' + } + + void testOperatorRenameAssignVariant() { + assertScript ''' + import groovy.transform.OperatorRename + + class Bag { + int total = 0 + void addInPlace(int n) { total += n } + int add(int n) { total + n } + } + + @OperatorRename(plus="add", plusAssign="addInPlace") + def exercise() { + def b = new Bag() + b += 5 + assert b.total == 5 + def s = b + 3 + assert s == 8 + assert b.total == 5 + } + exercise() + ''' + } + + void testOperatorRenameAssignExpressionValueIsReceiver() { + assertScript ''' + import groovy.transform.OperatorRename + + class Bag { + int total = 0 + void addInPlace(int n) { total += n } + } + + @OperatorRename(plusAssign="addInPlace") + def exercise() { + def b = new Bag() + def r = (b += 5) + assert b.total == 5 + assert r.is(b) + } + exercise() + ''' + } + + void testOperatorRenameBaseOnlyKeepsAssignWrap() { + assertScript ''' + import groovy.transform.OperatorRename + + class Bag { + int total = 0 + Bag add(int n) { def b = new Bag(); b.total = total + n; b } + } + + @OperatorRename(plus="add") + def exercise() { + def b = new Bag() + b += 5 + assert b.total == 5 + } + exercise() + ''' + } + + void testStaticAllTwelveOperators() { + assertScript ''' + import groovy.transform.CompileStatic + + @CompileStatic + class Probe { + String last = '' + void plusAssign(int n) { last = 'plusAssign' } + void minusAssign(int n) { last = 'minusAssign' } + void multiplyAssign(int n) { last = 'multiplyAssign' } + void divAssign(int n) { last = 'divAssign' } + void remainderAssign(int n) { last = 'remainderAssign' } + void powerAssign(int n) { last = 'powerAssign' } + void leftShiftAssign(int n) { last = 'leftShiftAssign' } + void rightShiftAssign(int n) { last = 'rightShiftAssign' } + void rightShiftUnsignedAssign(int n) { last = 'rightShiftUnsignedAssign' } + void andAssign(int n) { last = 'andAssign' } + void orAssign(int n) { last = 'orAssign' } + void xorAssign(int n) { last = 'xorAssign' } + } + + @CompileStatic + void exercise() { + def p = new Probe() + p += 1; assert p.last == 'plusAssign' + p -= 1; assert p.last == 'minusAssign' + p *= 1; assert p.last == 'multiplyAssign' + p /= 1; assert p.last == 'divAssign' + p %= 1; assert p.last == 'remainderAssign' + p **= 1; assert p.last == 'powerAssign' + p <<= 1; assert p.last == 'leftShiftAssign' + p >>= 1; assert p.last == 'rightShiftAssign' + p >>>= 1; assert p.last == 'rightShiftUnsignedAssign' + p &= 1; assert p.last == 'andAssign' + p |= 1; assert p.last == 'orAssign' + p ^= 1; assert p.last == 'xorAssign' + } + exercise() + ''' + } +}