[
https://issues.apache.org/jira/browse/GROOVY-11770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18076851#comment-18076851
]
ASF GitHub Bot commented on GROOVY-11770:
-----------------------------------------
Copilot commented on code in PR #2498:
URL: https://github.com/apache/groovy/pull/2498#discussion_r3154481414
##########
src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java:
##########
@@ -273,11 +278,15 @@ private static ClassNode
parameterizeLowestUpperBound(final ClassNode lub, final
if (areEqualWithGenerics(t1, isPrimitiveType(a)?getWrapper(a):a)
&& areEqualWithGenerics(t2, isPrimitiveType(b)?getWrapper(b):b)) {
// "String implements Comparable<String>" and "StringBuffer
implements Comparable<StringBuffer>"
basicType = fallback; // do not loop
+ } else if (ctx.isExpanding(t1, t2)) {
+ // GROOVY-11770: structural cycle (e.g. LUB(B, D) where B
extends A<W<B>>, D extends A<W<D>>)
Review Comment:
The comment says “structural cycle”, but the guard currently detects cycles
using identity (`TypePair` uses `==`). Either adjust the wording to reflect
identity-based detection, or change the keying strategy to something
structural/stable so the comment matches the behavior.
```suggestion
// GROOVY-11770: recursion guard for an already-expanding
type pair (e.g. LUB(B, D) where B extends A<W<B>>, D extends A<W<D>>)
```
##########
src/test/groovy/groovy/transform/stc/GenericsSTCTest.groovy:
##########
@@ -4616,6 +4616,26 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase
{
}
}
+ // GROOVY-11770
+ @Test
+ void testNoStackOverflow3() {
+ // Cycle through a wrapper type: LUB(B, D) needs LUB(W<B>, W<D>),
+ // which needs LUB(B, D) again. Caught by structural cycle guard.
Review Comment:
This test comment says the cycle is “caught by structural cycle guard”, but
the new guard in `WideningCategories` is identity-based (and may not be
structural if new `ClassNode` instances are created). Suggest rewording the
comment to match the implementation (e.g., “caught by in-flight LUB pair
guard”).
```suggestion
// which needs LUB(B, D) again. Caught by the in-flight LUB pair
guard.
```
##########
src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java:
##########
@@ -273,11 +278,15 @@ private static ClassNode
parameterizeLowestUpperBound(final ClassNode lub, final
if (areEqualWithGenerics(t1, isPrimitiveType(a)?getWrapper(a):a)
&& areEqualWithGenerics(t2, isPrimitiveType(b)?getWrapper(b):b)) {
// "String implements Comparable<String>" and "StringBuffer
implements Comparable<StringBuffer>"
basicType = fallback; // do not loop
+ } else if (ctx.isExpanding(t1, t2)) {
+ // GROOVY-11770: structural cycle (e.g. LUB(B, D) where B
extends A<W<B>>, D extends A<W<D>>)
+ basicType = fallback;
} else {
+ ctx.enter(t1, t2);
try {
- basicType = lowestUpperBound(t1, t2);
- } catch (StackOverflowError ignore) {
- basicType = fallback; // best we can do for now
+ basicType = lowestUpperBound(ctx, t1, t2);
Review Comment:
`LowestUpperBoundContext` uses identity to track in-flight `(t1, t2)` pairs.
However, generics rewriting utilities (e.g.,
`correctToGenericsSpecRecurse`/`makeClassSafe0`) can produce fresh `ClassNode`
instances for the same logical type, which can defeat identity-based cycle
detection; with the `StackOverflowError` catch removed, a missed cycle could
reintroduce a hard SOE. Consider keeping the SOE catch as a last-resort
backstop, or normalizing the tracked pair to a stable representation (redirect
+ generics signature) so logically-equal nodes match.
```suggestion
basicType = lowestUpperBound(ctx, t1, t2);
} catch (StackOverflowError ignore) {
// Some generics-rewriting paths can create fresh
ClassNode instances for the same
// logical type, which may defeat identity-based cycle
tracking in the context.
// Fall back rather than allowing an unbounded recursive
expansion to overflow.
basicType = fallback;
```
> StackOverflowError processing generics for kubernetes-client library
> --------------------------------------------------------------------
>
> Key: GROOVY-11770
> URL: https://issues.apache.org/jira/browse/GROOVY-11770
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
> Fix For: 4.0.29, 5.0.2
>
>
> When processing a file using the kubernetes-client library, the Groovy
> compiler recurses endlessly parsing the generics.
> Repo:
> https://github.com/paulk-asert/kubernetes_client_stackoverflow
> This is just a reproducer which excludes Grails from this Grails issue/repo:
> https://github.com/apache/grails-core/issues/15082
> https://github.com/jdaugherty/grails-issue-stackoverflow-on-compile
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
