blackdrag commented on code in PR #2496:
URL: https://github.com/apache/groovy/pull/2496#discussion_r3159976554
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java:
##########
@@ -1789,9 +1794,22 @@ private static GenericsType applyGenericsContext(final
Map<GenericsTypeName, Gen
GenericsType specType = spec.get(name);
if (specType != null) return specType;
if (hasNonTrivialBounds(gt)) {
- GenericsType newGT = new GenericsType(type,
applyGenericsContext(spec, gt.getUpperBounds()), applyGenericsContext(spec,
gt.getLowerBound()));
- newGT.setPlaceholder(true);
- return newGT;
+ // GROOVY-11022: avoid infinite recursion on F-bounded type
+ // parameters whose self-reference appears inside a wildcard
+ // bound (e.g. `<K extends Comparable<? super K>>`)
+ Deque<GenericsTypeName> expanding = EXPANDING_BOUNDS.get();
+ if (expanding.contains(name)) return gt;
+ expanding.push(name);
+ try {
+ GenericsType newGT = new GenericsType(type,
applyGenericsContext(spec, gt.getUpperBounds()), applyGenericsContext(spec,
gt.getLowerBound()));
+ newGT.setPlaceholder(true);
+ return newGT;
+ } finally {
+ expanding.pop();
Review Comment:
so we push to expanding and then pop from expanding. That means we remove
the entry we just added, even in the case of line 1806, with the return. Why is
it then not always empty again? Ah, because of the recursive call
applyGenericsContext(spec, gt.getLowerBound()). But why does it have to be a
ThreadLocal then? I would prefere going for a breaking change here nstead of
using ThreadLocal
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java:
##########
@@ -1769,6 +1771,9 @@ static GenericsType[] getGenericsWithoutArray(final
ClassNode type) {
return type.getGenericsTypes();
}
+ private static final ThreadLocal<Deque<GenericsTypeName>> EXPANDING_BOUNDS
=
+ ThreadLocal.withInitial(ArrayDeque::new);
Review Comment:
I am very critical to using a ThreadLocal. At the very least there should be
a description of why this has to be ThreadLocal
--
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]
