[
https://issues.apache.org/jira/browse/GROOVY-12036?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18083276#comment-18083276
]
ASF GitHub Bot commented on GROOVY-12036:
-----------------------------------------
Copilot commented on code in PR #2558:
URL: https://github.com/apache/groovy/pull/2558#discussion_r3297866065
##########
src/main/java/org/codehaus/groovy/runtime/StreamGroovyMethods.java:
##########
@@ -692,16 +722,32 @@ public static <T> T[] toArray(final Stream<? extends T>
self, final Class<T> typ
}
/**
- * Accumulates the elements of stream into a new List.
+ * Accumulates the elements of stream into a new mutable List.
+ * <p>
+ * <strong>Note:</strong> since JDK 16, {@link Stream} has a native
+ * {@code toList()} method that returns an <em>unmodifiable</em> list.
+ * Java instance methods take precedence over GDK extensions in both
+ * {@code @CompileStatic} and dynamic dispatch, so {@code stream.toList()}
+ * now resolves to the native call and yields an immutable result —
+ * <em>not</em> a fresh {@code ArrayList} as it did pre-JDK 16.
+ * Direct callers of this extension method are pointed at the explicit
Review Comment:
The Javadoc here states that pre-JDK 16 this extension returned a “fresh
ArrayList” and that this method returns a “new mutable List”.
Collectors.toList() doesn’t specify ArrayList nor mutability, so this is
stronger than the implementation contract. Either weaken the wording (e.g.
“typically mutable”) or switch implementation to a collector that guarantees
ArrayList/mutability.
##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -15870,6 +15911,23 @@ public static <T> Set<T> toSet(Iterator<T> self) {
return answer;
}
+ /**
+ * Convert an iterator to an immutable Set. The iterator will become
+ * exhausted of elements after making this conversion. Returns the
+ * canonical empty set ({@code Set.of()}) when the iterator is empty.
+ *
+ * @param self an iterator
+ * @return an immutable Set of the elements from the iterator
+ * @since 6.0.0
+ */
+ public static <T> Set<T> toImmutableSet(Iterator<T> self) {
+ Set<T> answer = new HashSet<>();
+ while (self.hasNext()) {
+ answer.add(self.next());
+ }
+ return Set.copyOf(answer);
Review Comment:
Same issue as toImmutableList(Iterator): Set.copyOf rejects null elements,
so this will throw NullPointerException if the iterator yields null. Either
document the restriction or use an unmodifiable wrapper over a copied Set to
keep behavior consistent with other Groovy “immutable” helpers.
##########
src/main/java/org/codehaus/groovy/runtime/StreamGroovyMethods.java:
##########
@@ -49,6 +50,35 @@ public class StreamGroovyMethods {
private StreamGroovyMethods() {
}
+ // --
> GDK: cache Collectors instances in StreamGroovyMethods and
> ParallelCollectionExtensions
> ---------------------------------------------------------------------------------------
>
> Key: GROOVY-12036
> URL: https://issues.apache.org/jira/browse/GROOVY-12036
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Cache Collectors.toList() and Collectors.toSet() as static singletons in
> StreamGroovyMethods and ParallelCollectionExtensions, eliminating per-call
> collector allocation in 9 GDK call sites.
> A benefit in Eclipse Collections as noted here:
> https://donraab.medium.com/counting-and-collecting-collectors-d69b7c9aaca0
> Groovy is not a collections library but this is a very minor improvement with
> no downside.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
