Copilot commented on code in PR #2497:
URL: https://github.com/apache/groovy/pull/2497#discussion_r3154041200
##########
src/test/groovy/groovy/transform/stc/GenericsSTCTest.groovy:
##########
@@ -4616,6 +4616,40 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase
{
}
}
+ // GROOVY-11022
+ @Test
+ void testNoStackOverflow3() {
+ // F-bounded type parameter with self-reference inside a `? super`
wildcard
+ assertScript '''
+ class C<K, V> {
+ public static <K extends Comparable<? super K>, V> C<K, V> m(K
k, V v) {
+ new C<K, V>()
+ }
+ }
+ class Main {
+ @groovy.transform.TypeChecked
+ static test() {
+ C.<Object, Integer>m(null, 1)
Review Comment:
In this `@TypeChecked` script, the explicit type argument `Object` does not
satisfy the method bound `K extends Comparable<? super K>` (since Object is not
Comparable). This risks the test failing with a bounds/type-checking error (or
masking the intended regression) depending on how generic bounds are validated.
Consider using a type that satisfies the bound (e.g., `Integer`/`String`) while
still exercising the recursive wildcard structure.
```suggestion
C.<String, Integer>m(null, 1)
```
##########
src/test/groovy/groovy/transform/stc/GenericsSTCTest.groovy:
##########
@@ -4616,6 +4616,40 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase
{
}
}
+ // GROOVY-11022
+ @Test
+ void testNoStackOverflow3() {
+ // F-bounded type parameter with self-reference inside a `? super`
wildcard
+ assertScript '''
+ class C<K, V> {
+ public static <K extends Comparable<? super K>, V> C<K, V> m(K
k, V v) {
+ new C<K, V>()
+ }
+ }
+ class Main {
+ @groovy.transform.TypeChecked
+ static test() {
+ C.<Object, Integer>m(null, 1)
+ }
+ }
+ Main.test()
+ '''
+
+ // self-reference inside a `? extends` wildcard
+ assertScript '''
+ class C<K> {
+ public static <K extends Iterable<? extends K>> C<K> m(K k) {
+ new C<K>()
+ }
+ }
+ @groovy.transform.TypeChecked
+ void test() {
+ C.<List<Object>>m(null)
Review Comment:
The explicit type argument `List<Object>` does not appear to satisfy the
bound `K extends Iterable<? extends K>` (a `List<Object>` is
`Iterable<Object>`, not `Iterable<? extends List<Object>>`). To keep this
regression focused on the StackOverflowError and avoid potential
bounds-checking failures, consider introducing a small self-referential type
(e.g., a class implementing `Iterable<Self>`) and using that as `K`.
```suggestion
class Self implements Iterable<Self> {
@Override
Iterator<Self> iterator() {
Collections.<Self>emptyList().iterator()
}
}
@groovy.transform.TypeChecked
void test() {
C.<Self>m(null)
```
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java:
##########
@@ -1841,26 +1882,38 @@ private static boolean hasNonTrivialBounds(final
GenericsType gt) {
}
static ClassNode[] applyGenericsContext(final Map<GenericsTypeName,
GenericsType> spec, final ClassNode[] types) {
+ if (types == null) return null;
+ return applyGenericsContext(new BoundExpansionContext(), spec, types);
+ }
+
+ private static ClassNode[] applyGenericsContext(final
BoundExpansionContext ctx, final Map<GenericsTypeName, GenericsType> spec,
final ClassNode[] types) {
if (types == null) return null;
final int nTypes = types.length;
ClassNode[] newTypes = new ClassNode[nTypes];
for (int i = 0; i < nTypes; i += 1) {
- newTypes[i] = applyGenericsContext(spec, types[i]);
+ newTypes[i] = applyGenericsContext(ctx, spec, types[i]);
}
return newTypes;
}
static ClassNode applyGenericsContext(final Map<GenericsTypeName,
GenericsType> spec, final ClassNode type) {
+ if (type == null || !isUsingGenericsOrIsArrayUsingGenerics(type)) {
+ return type;
+ }
+ return applyGenericsContext(new BoundExpansionContext(), spec, type);
+ }
Review Comment:
`applyGenericsContext(spec, type)` now allocates a new
`BoundExpansionContext` unconditionally for any generic-using type, even when
`spec` is null/empty (in which case the context is never used). Since this
method is on a hot path during STC, consider creating/passing the context only
when `asBoolean(spec)` is true and you are about to expand generics
placeholders/bounds, to avoid per-call allocation overhead in the common "no
substitutions" case.
--
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]
