[
https://issues.apache.org/jira/browse/GROOVY-11964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18076278#comment-18076278
]
ASF GitHub Bot commented on GROOVY-11964:
-----------------------------------------
Copilot commented on code in PR #2489:
URL: https://github.com/apache/groovy/pull/2489#discussion_r3142882307
##########
src/main/java/org/codehaus/groovy/runtime/MultipleAssignmentSupport.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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 org.codehaus.groovy.runtime;
+
+import groovy.lang.IntRange;
+
+import java.lang.reflect.Array;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Runtime helpers used by the GEP-20 multi-assignment destructuring bytecode.
+ *
+ * <p>These are called from generated bytecode only. User code should not rely
on them.</p>
+ */
+public final class MultipleAssignmentSupport {
+
+ private MultipleAssignmentSupport() {}
+
+ private static final Class<?>[] INT_RANGE_PARAM = { IntRange.class };
+
+ /**
+ * Resolve the value for a tail rest binding ({@code *t}) at declarator
index
+ * {@code fromIndex}. The GEP-20 dispatch model has three paths:
+ *
+ * <ul>
+ * <li><b>Path A</b> — {@code Stream<T>}: wrap the already-advanced
iterator
+ * in a fresh sequential Stream and chain {@code onClose} to the
+ * original. The new Stream reports no spliterator characteristics
+ * (preserving them would require reading the original spliterator,
+ * which is mutually exclusive with the iterator path used for head
+ * extraction).</li>
+ * <li><b>Path B</b> — receiver with a resolvable {@code
getAt(IntRange)}.
+ * Implemented as fast-path {@code instanceof} branches for
+ * {@code List}, {@code CharSequence}, and Java arrays (size-aware so
+ * out-of-bounds {@code fromIndex} returns the canonical empty value
+ * without calling user code), with an MOP-dispatched fallback for
+ * any other Path B receiver — {@link java.util.BitSet} (returns
+ * {@code BitSet}), user custom classes, etc.</li>
+ * <li><b>Path C</b> — anything else iterable: return the
already-advanced
+ * iterator.</li>
+ * </ul>
+ *
+ * <p>The actual {@code instanceof} checks are ordered for performance
+ * (List/CharSequence/array fast paths first, then Stream, then MOP
+ * fallback, then iterator) but the conceptual dispatch order is A, then
+ * B, then C.</p>
+ *
+ * <p>Primitive streams ({@code IntStream}, {@code LongStream},
+ * {@code DoubleStream}) are not subtypes of {@code Stream<T>} and
therefore
+ * fall through to Path B (if they expose {@code getAt(IntRange)}) or
+ * Path C; their elements are boxed by the underlying iterator.</p>
+ *
+ * @param rhs the original RHS value
+ * @param fromIndex the declarator position of the rest binding
+ * @param seq the iterator that has already yielded the preceding
fixed-slot values
+ */
+ public static Object tailRest(final Object rhs, final int fromIndex, final
Iterator<?> seq) {
+ if (rhs instanceof List) {
+ if (fromIndex >= ((List<?>) rhs).size()) return
Collections.emptyList();
Review Comment:
In the List fast-path, returning Collections.emptyList() when fromIndex is
past the end makes the rest binding immutable. This is inconsistent with
Groovy's List range slicing (getAt(EmptyRange) / getAt(Range)) which returns a
new, mutable list (often of a similar concrete type). Consider returning a
mutable empty list (or delegating to List#getAt with an empty range) so `*t`
behaves the same whether it absorbs 0 or >0 elements.
```suggestion
if (fromIndex >= ((List<?>) rhs).size()) return new
java.util.ArrayList<>();
```
##########
src/main/java/org/codehaus/groovy/runtime/MultipleAssignmentSupport.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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 org.codehaus.groovy.runtime;
+
+import groovy.lang.IntRange;
+
+import java.lang.reflect.Array;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Runtime helpers used by the GEP-20 multi-assignment destructuring bytecode.
+ *
+ * <p>These are called from generated bytecode only. User code should not rely
on them.</p>
+ */
+public final class MultipleAssignmentSupport {
+
+ private MultipleAssignmentSupport() {}
+
+ private static final Class<?>[] INT_RANGE_PARAM = { IntRange.class };
+
+ /**
+ * Resolve the value for a tail rest binding ({@code *t}) at declarator
index
+ * {@code fromIndex}. The GEP-20 dispatch model has three paths:
+ *
+ * <ul>
+ * <li><b>Path A</b> — {@code Stream<T>}: wrap the already-advanced
iterator
+ * in a fresh sequential Stream and chain {@code onClose} to the
+ * original. The new Stream reports no spliterator characteristics
+ * (preserving them would require reading the original spliterator,
+ * which is mutually exclusive with the iterator path used for head
+ * extraction).</li>
+ * <li><b>Path B</b> — receiver with a resolvable {@code
getAt(IntRange)}.
+ * Implemented as fast-path {@code instanceof} branches for
+ * {@code List}, {@code CharSequence}, and Java arrays (size-aware so
+ * out-of-bounds {@code fromIndex} returns the canonical empty value
+ * without calling user code), with an MOP-dispatched fallback for
+ * any other Path B receiver — {@link java.util.BitSet} (returns
+ * {@code BitSet}), user custom classes, etc.</li>
+ * <li><b>Path C</b> — anything else iterable: return the
already-advanced
+ * iterator.</li>
+ * </ul>
+ *
+ * <p>The actual {@code instanceof} checks are ordered for performance
+ * (List/CharSequence/array fast paths first, then Stream, then MOP
+ * fallback, then iterator) but the conceptual dispatch order is A, then
+ * B, then C.</p>
+ *
+ * <p>Primitive streams ({@code IntStream}, {@code LongStream},
+ * {@code DoubleStream}) are not subtypes of {@code Stream<T>} and
therefore
+ * fall through to Path B (if they expose {@code getAt(IntRange)}) or
+ * Path C; their elements are boxed by the underlying iterator.</p>
+ *
+ * @param rhs the original RHS value
+ * @param fromIndex the declarator position of the rest binding
+ * @param seq the iterator that has already yielded the preceding
fixed-slot values
+ */
+ public static Object tailRest(final Object rhs, final int fromIndex, final
Iterator<?> seq) {
+ if (rhs instanceof List) {
+ if (fromIndex >= ((List<?>) rhs).size()) return
Collections.emptyList();
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs instanceof CharSequence) {
+ if (fromIndex >= ((CharSequence) rhs).length()) return "";
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs != null && rhs.getClass().isArray()) {
+ // Match the non-empty branch: ArrayGroovyMethods.getAt(T[],
IntRange) returns List<T>,
+ // so the empty case must be Collections.emptyList() — not an
empty T[] — for typed
+ // rest binders like `List<T> *t` to round-trip consistently.
+ if (fromIndex >= Array.getLength(rhs)) return
Collections.emptyList();
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs instanceof Stream) {
+ Spliterator<?> spliterator =
Spliterators.spliteratorUnknownSize(seq, 0);
+ Stream<?> tail = StreamSupport.stream(spliterator, false);
+ return tail.onClose(((Stream<?>) rhs)::close);
+ }
+ // MOP fallback: any other receiver with getAt(IntRange) — BitSet,
user classes, etc.
+ // Picked deliberately AFTER the Stream branch so a Stream RHS never
materialises via
+ // StreamGroovyMethods.getAt(Stream, IntRange).
+ if (rhs != null && InvokerHelper.getMetaClass(rhs).pickMethod("getAt",
INT_RANGE_PARAM) != null) {
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ return seq;
+ }
+
+ /**
+ * Resolve the value for a head or middle rest binding. Requires a sized,
indexable RHS
+ * (List, CharSequence, or array — or any other receiver with a
non-reversing
+ * {@code getAt(IntRange)}); for other RHS types this routes through the
MOP so that:
+ *
+ * <ul>
+ * <li>Truly non-indexable sources (iterators, sets, custom Iterables
without a
+ * {@code getAt(IntRange)} extension) fail fast with
+ * {@code MissingMethodException} — preventing silent
materialisation of
+ * unbounded sources.</li>
+ * <li>{@code Stream} RHS resolves {@code
StreamGroovyMethods.getAt(Stream, IntRange)},
+ * which rejects the destructuring slice's reverse range with
+ * {@code IllegalArgumentException("reverse range")} — also
fail-fast, also
+ * no materialisation. (Head/middle rest from {@code Stream} is
rejected at
+ * compile time under {@code @CompileStatic}; the IAE only surfaces
in
+ * dynamic mode.)</li>
+ * </ul>
+ *
+ * <p>When the RHS is shorter than the sum of fixed slots around the rest,
the computed range
+ * is inverted. DGM's {@code getAt(IntRange)} would reverse an inverted
range; for destructuring
+ * we want an empty slice instead, so this helper short-circuits that case
for the sized fast
+ * paths (List/CharSequence/array). Other indexable receivers are
responsible for their own
+ * bounds handling.</p>
+ *
+ * @param rhs the original RHS value
+ * @param fromIndex positive index where the slice begins
+ * @param toIndex negative index where the slice ends (inclusive)
+ */
+ public static Object nonTailRestSlice(final Object rhs, final int
fromIndex, final int toIndex) {
+ int size = -1;
+ if (rhs instanceof List) size = ((List<?>) rhs).size();
+ else if (rhs instanceof CharSequence) size = ((CharSequence)
rhs).length();
+ else if (rhs != null && rhs.getClass().isArray()) size =
Array.getLength(rhs);
+
+ if (size >= 0) {
+ int effectiveTo = toIndex < 0 ? size + toIndex : toIndex;
+ if (fromIndex > effectiveTo || fromIndex >= size || effectiveTo <
0) {
+ // Array empty-slice falls through to Collections.emptyList():
the non-empty
+ // path delegates to getAt(IntRange) which returns List<T> for
arrays, so the
+ // empty path must agree — see tailRest for the same alignment.
+ if (rhs instanceof CharSequence) return "";
+ return Collections.emptyList();
+ }
Review Comment:
nonTailRestSlice returns Collections.emptyList() for sized non-CharSequence
RHS when the computed slice is empty/inverted. For List/array receivers,
Groovy's getAt(EmptyRange) returns a new mutable list; returning an immutable
empty list here makes the rest binding's mutability depend on whether it
captured 0 elements. Consider returning a mutable empty list (and ideally
matching Groovy's usual empty-slice behavior) to avoid
UnsupportedOperationException surprises.
> Additional multi-assignment forms
> ---------------------------------
>
> Key: GROOVY-11964
> URL: https://issues.apache.org/jira/browse/GROOVY-11964
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> Three idioms are common in modern languages but unavailable in Groovy:
> * Tail rest binding: {{def (h, *t) = list}} - separate the head from the
> remainder. Common in functional code (recursive list traversal,
> pipeline-style processing of streams and iterators).
> * Map-style destructuring: {{def (name: n, age: a) = person}} - extract named
> properties or map keys directly, without intermediate variables.
> * Rest in non-tail positions: {{def (*f, last) = list, def (l, *m, r) =
> list}} - useful when the boundaries are at the ends rather than the middle.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
