daniellansun commented on code in PR #3259:
URL: https://github.com/apache/groovy/pull/3259#discussion_r4550574203
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -368,6 +368,89 @@ public static ClassNode erasure(final ClassNode type) {
return t.getPlainNodeReference();
}
+ /**
+ * Type arguments safe to store on {@code owner}. A class that declares no
+ * type parameters gets none. Each argument is a reference type.
+ *
+ * @param owner the parameterized type
+ * @param arguments the arguments about to be stored
+ * @return the arguments to store, or {@code null} when the class declares
none
+ */
+ public static GenericsType[] legalTypeArguments(final ClassNode owner,
final GenericsType[] arguments) {
+ if (arguments == null) return null;
+ if (arguments.length > 0 && declaresNoTypeParameters(owner)) return
null;
+ GenericsType[] legal = new GenericsType[arguments.length];
+ boolean changed = false;
+ for (int i = 0; i < arguments.length; i += 1) {
+ legal[i] = asReferenceTypeArgument(arguments[i]);
+ changed |= legal[i] != arguments[i];
+ }
+ return changed ? legal : arguments;
+ }
+
+ private static boolean declaresNoTypeParameters(final ClassNode type) {
+ if (type == null || type.isGenericsPlaceHolder()) return false;
+ ClassNode declared = type.redirect();
Review Comment:
Agreed. `ClassNode.redirect()` returns this node when it is not a proxy, so
the null check is gone.
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -368,6 +368,89 @@ public static ClassNode erasure(final ClassNode type) {
return t.getPlainNodeReference();
}
+ /**
+ * Type arguments safe to store on {@code owner}. A class that declares no
+ * type parameters gets none. Each argument is a reference type.
+ *
+ * @param owner the parameterized type
+ * @param arguments the arguments about to be stored
+ * @return the arguments to store, or {@code null} when the class declares
none
+ */
+ public static GenericsType[] legalTypeArguments(final ClassNode owner,
final GenericsType[] arguments) {
+ if (arguments == null) return null;
+ if (arguments.length > 0 && declaresNoTypeParameters(owner)) return
null;
+ GenericsType[] legal = new GenericsType[arguments.length];
+ boolean changed = false;
+ for (int i = 0; i < arguments.length; i += 1) {
+ legal[i] = asReferenceTypeArgument(arguments[i]);
+ changed |= legal[i] != arguments[i];
+ }
+ return changed ? legal : arguments;
+ }
+
+ private static boolean declaresNoTypeParameters(final ClassNode type) {
+ if (type == null || type.isGenericsPlaceHolder()) return false;
+ ClassNode declared = type.redirect();
+ if (declared == null || !declared.isResolved() || declared.isArray())
return false;
Review Comment:
Whether a class declares type parameters now comes from the declaration, not
from whether the use site has already been resolved. A primary source class
uses the formals on that node. A loaded class uses `Class.getTypeParameters()`
when the formals were not copied onto the node. An unresolved use site is not
treated as a declaration, so this method does not strip its arguments;
`ResolveVisitor` applies `legalTypeArguments` after the redirect is set.
##########
src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java:
##########
@@ -493,19 +493,26 @@ static void addSubTypes(StringBuilder ret, GenericsType[]
types, String start, S
if (type.getType().isArray()) {
ret.append("[");
addSubTypes(ret, new GenericsType[]{new
GenericsType(type.getType().getComponentType())}, "", "");
+ } else if ("<".equals(start) && isPrimitiveType(type.getType()) &&
!isPrimitiveVoid(type.getType())) {
+ // Last resort (JVMS 4.7.9.1): a type argument that is still a
+ // primitive is written as its wrapper, which is what javac
writes
+ // for the legal reference type. Parameters and return types
use a
+ // different start marker and stay primitive.
+ writeGenericsBoundType(ret,
ClassHelper.getWrapper(type.getType()), false);
Review Comment:
Agreed — this was mixing concerns. `BytecodeHelper` no longer boxes a
primitive type argument or a `? extends int` bound. Boxing is done in
`GenericsUtils.asReferenceTypeArgument` when the `GenericsType` is constructed.
The inferred call type for `of(1, 2)` and `ext(1)` is `Integer`, and the method
signatures do not contain `<I` or `+I`.
##########
src/main/java/org/codehaus/groovy/classgen/asm/TypeSignatureWriter.java:
##########
@@ -46,7 +47,47 @@ static void writeParameterizedClass(final StringBuilder ret,
final ClassNode pri
return;
}
ret.append(BytecodeHelper.getTypeDescription(printType, false));
- BytecodeHelper.addSubTypes(ret, printType.getGenericsTypes(), "<",
">");
+ BytecodeHelper.addSubTypes(ret, typeArguments(printType), "<", ">");
+ }
+
+ /**
+ * Type arguments written for {@code printType}. Last resort for JVMS
+ * 4.7.9.1: if a use site still names arguments a class does not declare,
+ * write the raw type javac would emit for the legal type. The type
+ * checker is what must keep {@code Serializable<T>} and {@code
Queryable<int>}
+ * out of the AST.
+ */
+ private static GenericsType[] typeArguments(final ClassNode printType) {
+ GenericsType[] useSite = printType.getGenericsTypes();
+ if (useSite == null || useSite.length == 0) {
+ return useSite;
+ }
+ int declared = declaredTypeParameters(printType);
+ if (declared >= 0 && declared != useSite.length) {
+ return null;
+ }
+ return useSite;
+ }
+
+ /**
+ * Number of type parameters the class actually declares, or {@code -1}
+ * when that is not known yet. An unresolved {@code java.*} name is loaded
+ * so a use site cannot attach arguments to {@code Serializable}.
+ */
+ private static int declaredTypeParameters(final ClassNode printType) {
+ ClassNode resolved = printType.isResolved() ? printType :
printType.redirect();
Review Comment:
Agreed. At signature write time the declaration is read from the class node.
Unresolved names are not loaded here.
##########
src/main/java/org/codehaus/groovy/classgen/asm/TypeSignatureWriter.java:
##########
@@ -46,7 +47,47 @@ static void writeParameterizedClass(final StringBuilder ret,
final ClassNode pri
return;
}
ret.append(BytecodeHelper.getTypeDescription(printType, false));
- BytecodeHelper.addSubTypes(ret, printType.getGenericsTypes(), "<",
">");
+ BytecodeHelper.addSubTypes(ret, typeArguments(printType), "<", ">");
+ }
+
+ /**
+ * Type arguments written for {@code printType}. Last resort for JVMS
+ * 4.7.9.1: if a use site still names arguments a class does not declare,
+ * write the raw type javac would emit for the legal type. The type
+ * checker is what must keep {@code Serializable<T>} and {@code
Queryable<int>}
+ * out of the AST.
+ */
+ private static GenericsType[] typeArguments(final ClassNode printType) {
+ GenericsType[] useSite = printType.getGenericsTypes();
+ if (useSite == null || useSite.length == 0) {
+ return useSite;
+ }
+ int declared = declaredTypeParameters(printType);
+ if (declared >= 0 && declared != useSite.length) {
+ return null;
+ }
+ return useSite;
+ }
+
+ /**
+ * Number of type parameters the class actually declares, or {@code -1}
+ * when that is not known yet. An unresolved {@code java.*} name is loaded
+ * so a use site cannot attach arguments to {@code Serializable}.
+ */
+ private static int declaredTypeParameters(final ClassNode printType) {
+ ClassNode resolved = printType.isResolved() ? printType :
printType.redirect();
+ if (resolved != null && resolved.isResolved() && !resolved.isArray()) {
+ return resolved.getTypeClass().getTypeParameters().length;
+ }
+ String name = printType.getName();
+ if (name != null && (name.startsWith("java.") ||
name.startsWith("javax.") || name.startsWith("groovy."))) {
+ try {
+ return Class.forName(name, false,
TypeSignatureWriter.class.getClassLoader()).getTypeParameters().length;
Review Comment:
Agreed. That belongs in `ResolveVisitor`. Source use sites such as
`Serializable<T>` and `Raw<String>` are corrected once the class is known, and
the field signature is the raw type. The writer no longer uses `Class.forName`,
and unresolved names are not loaded there.
A type invented later is also kept legal in the AST. The lowest upper bound
of a multi-key group value is still named `java.io.Serializable` (the
compile-time class), but it is no longer parameterized: `Comparable`'s type
argument stays on `Comparable`. The signature is the raw type because the AST
type is raw, not because the writer drops an illegal argument.
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/runtime/Queryable.java:
##########
@@ -516,6 +539,52 @@ default Queryable<T> union(Queryable<? extends T>
queryable) {
*/
List<T> toList();
+ /**
+ * Iterates the materialized elements. A static {@code for} loop calls this
+ * rather than wrapping the query as a single value.
+ *
+ * @return an iterator over the elements
+ * @since 7.0.0
+ */
+ default Iterator<T> iterator() {
+ return toList().iterator();
+ }
+
+ /**
+ * Coerces this query. {@link Queryable}, {@link List}, {@link Collection}
and
+ * {@link Iterable} are answered here. Every other target is delegated to
+ * {@link DefaultGroovyMethods#asType(Object, Class)}, which throws
+ * {@link GroovyCastException} when the value is not compatible with the
target.
+ *
+ * @param type the requested type
+ * @return the coerced value
+ * @throws GroovyCastException when {@code type} is null or the value
cannot be coerced
+ * @since 7.0.0
+ */
+ default Object asType(Class<?> type) {
+ if (type == null) {
+ throw new GroovyCastException("Cannot cast '" +
getClass().getName() + "' to null");
+ }
+ if (type == Queryable.class) {
+ return this;
Review Comment:
`as List` is `toList()`. `Queryable` does not implement `Iterable` or
`Collection`, so `DefaultGroovyMethods.asType(Object, Class)` cannot walk the
elements and would throw for `Collection`, `Iterable`, and `Set`. Those go
through `DefaultGroovyMethods.asType(toList(), type)`. `as Queryable` returns
this query, and `as Foo` still throws `GroovyCastException`.
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/runtime/QueryableCollection.java:
##########
@@ -684,12 +690,128 @@ public long size() {
}
/**
- * Returns a reusable stream over the current elements.
+ * Returns a new stream over the current elements. Creating the stream
does not
+ * pull. An iterable source is read again by every stream. A one-shot
stream is
+ * pulled into a buffer as it is consumed, and a later call replays that
buffer
+ * instead of returning the stream that was already handed out.
*
- * @return the element stream
+ * @return a stream that has not been consumed
*/
@Override
public Stream<T> stream() {
+ writeLock.lock();
+ try {
+ if (sourceIterable != null) {
+ Stream<T> fresh = toStream(sourceIterable);
+ if (!fresh.isParallel() && isParallel()) {
+ fresh = fresh.parallel();
+ }
+ return fresh;
+ }
+ bindUpstream();
+ return StreamSupport.stream(ReplaySpliterator::new,
upstreamCharacteristics, false);
+ } finally {
+ writeLock.unlock();
+ }
+ }
+
+ /**
+ * Binds the one-shot source. The size is recorded before any element is
pulled,
+ * so a later {@code count} can use it without running projections.
+ */
+ private void bindUpstream() {
+ if (upstreamBound) {
+ return;
+ }
+ upstreamBound = true;
+ upstream = openStream().spliterator();
+ long exact = upstream.getExactSizeIfKnown();
+ upstreamExactSize = exact;
+ upstreamCharacteristics = Spliterator.ORDERED;
+ if (exact >= 0) {
+ upstreamCharacteristics |= Spliterator.SIZED |
Spliterator.SUBSIZED;
+ }
+ }
+
+ /**
+ * One cursor over a one-shot source. Elements are appended to {@link
#replay}
+ * as they are pulled, so another stream can read them from the start.
+ * A sized source reports that size, and {@code Stream.count} then does
not pull.
+ */
+ private final class ReplaySpliterator implements Spliterator<T> {
+ private int cursor;
+
+ @Override
+ public boolean tryAdvance(Consumer<? super T> action) {
+ writeLock.lock();
Review Comment:
Elements already in the replay buffer are served under the read lock. The
callback runs after that lock is released. The write lock is taken only when
the cursor is at the end of the buffer and the next element has to be pulled
from upstream. Two concurrent readers of one stream see the same sequence,
including nulls, with no lost or duplicated elements.
--
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]
