daniellansun opened a new pull request, #3283: URL: https://github.com/apache/groovy/pull/3283
Backport of https://github.com/apache/groovy/pull/3280 to `GROOVY_6_0_X`. Follow-up to https://github.com/apache/groovy/pull/3226 (already on this branch). ## Review follow-up The array-store duplicate lived on `BinaryExpressionMultiTypeDispatcher` as `duplicateTopBeneath(operandStack, operandsBelow)`. The only caller passed `2`. That helper now sits on `OperandStack` next to `dup` and `swap`, as `dupUnderTwo()`. An array store has already cast the index to `int`, so the two operands under the value are one word each (array reference, int index). The copy is `DUP_X2`, or `DUP2_X2` when the value is a `long` or a `double`. `DUP_X1` and `DUP2_X1` are not part of this operation. ## What still uses a temporary A plain array or `putAt` statement evaluates receiver, index and right-hand side as the store arguments. No temporary is allocated. When the assignment expression's value is required: - The array-store fast path duplicates the value under those two operands before the component cast. The store consumes the original. The expression value is the pre-cast right-hand side, not the value returned by `putAt`. - A `putAt` call cannot share that duplicate. The classic call site pushes a `CallSite` under the receiver and the index, three words below the value, and there is no `DUP_X3`. Inserting the copy only under the receiver and the index would make it the `invokeinterface` receiver. Invokedynamic has no call-site operand, but the copy would have to be emitted inside that backend after the arguments are on the stack. Both backends keep one temporary across the call and clear a reference slot immediately afterwards. Compound `+=`, Elvis `?=` / `??=`, and postfix `a[i]++` evaluate the receiver and the index once because each is used for the read and the write. The operation result is also stored, because `putAt` consumes it and the expression value is the operation result. Those slots are required. Reference slots are cleared once the value has been reloaded. `removeVar` only forgets the slot; it does not clear it. ## Guarantees - Evaluation order for a plain subscript assignment stays receiver, then index, then right-hand side. A safe subscript still evaluates the right-hand side before the null check and skips the index when the receiver is null. - The value of an assignment expression remains the right-hand side (or the compound-operation result), not the value returned by `putAt`. - A reference temporary that subscript assignment still needs does not stay reachable for the rest of the enclosing method. ## Tests - `bugs.Groovy10657` covers statement and expression `putAt`, array assignment, Elvis subscript, compound `+=`, and postfix `a[i]++`, including that a large right-hand side or receiver can be collected while the frame is still live. - `StaticCompilationTest` keeps the `@CompileStatic` store sequences (`intArraySet`, `objectArraySet`, `DUP_X2`, `DUP2_X2`). - `BinaryOperationsTest` checks the same store sequences without `@CompileStatic` on the classic backend, and the invokedynamic `putAt` shape: a statement allocates no right-hand slot, and an expression clears the one temporary it keeps. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
