Copilot commented on code in PR #2914:
URL: https://github.com/apache/groovy/pull/2914#discussion_r3969637208
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -1214,6 +1235,16 @@ private void propagateTemporaryTypeInfo(final
Map<Object, List<ClassNode>> lhs,
typeCheckingContext.peekTemporaryTypeInfo(v).add(newUnionTypeClassNode(types));
}
}
+
+ negatives.forEach(this::putNotInstanceOfTypeInfo);
+ }
+
+ private static void collectNegativeTypeInfo(final Map<Object,
List<ClassNode>> tti, final Map<Object, List<ClassNode>> negatives) {
+ for (var entry : tti.entrySet()) {
+ if (entry.getKey() instanceof Object[] arr) {
+ negatives.computeIfAbsent(arr[1], k -> new
ArrayList<>()).addAll(entry.getValue());
Review Comment:
`putNotInstanceOfTypeInfo` is being invoked with keys coming from `arr[1]`,
but the negative type info entries are originally keyed by an `Object[]` marker
(as evidenced by `k instanceof Object[]` filtering elsewhere). This looks
inconsistent with how negative `instanceof` information is
represented/recognized, and may cause the negative info to be ignored or
treated like a positive key (breaking else-branch intersection narrowing).
Consider preserving the original `Object[]`-key shape when merging (e.g., merge
by the underlying real key but reconstitute the negated key object used by
`putNotInstanceOfTypeInfo`), or introduce a dedicated helper that records
merged negative-type info in the canonical representation expected by the rest
of STC.
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -7682,7 +7754,25 @@ private ClassNode getInferredTypeFromTempInfo(final
Expression expression, final
}
private static ClassNode newIntersectionTypeClassNode(final
Collection<ClassNode> types) {
- Map<Boolean, List<ClassNode>> spec =
types.stream().collect(Collectors.partitioningBy(ClassNode::isInterface));
+ int i = 0; // GROOVY-12000: distribute over a union so each
alternative stands alone: T & (A | B) is (T & A) | (T & B)
+ for (ClassNode type : types) {
+ if (type instanceof UnionTypeClassNode union) {
+ List<ClassNode> alternatives = new ArrayList<>();
+ for (ClassNode delegate : union.getDelegates()) {
+ List<ClassNode> copy = new ArrayList<>(types);
+ copy.set(i, delegate);
+ ClassNode alternative = newIntersectionTypeClassNode(copy);
+ if (alternatives.stream().noneMatch(a ->
implementsInterfaceOrIsSubclassOf(alternative, a))) {
+ alternatives.removeIf(a ->
implementsInterfaceOrIsSubclassOf(a, alternative)); // subsumed
+ alternatives.add(alternative);
+ }
+ }
+ return newUnionTypeClassNode(alternatives);
+ }
+ i += 1;
+ }
Review Comment:
This distributes intersection over unions via recursion and list-copying. If
`types` can contain multiple union members (or nested unions), this can grow
combinatorially (cartesian product), increasing compile-time significantly. If
the intent is only to handle the common case of a single union in the
intersection, consider enforcing/short-circuiting to distribute only one union
occurrence (or flatten/normalize unions first), and/or add a guard to avoid
exponential expansion in more complex type graphs.
--
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]
