[
https://issues.apache.org/jira/browse/GROOVY-12000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113381#comment-18113381
]
ASF GitHub Bot commented on GROOVY-12000:
-----------------------------------------
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.
> STC: instanceof or combined with not
> ------------------------------------
>
> Key: GROOVY-12000
> URL: https://issues.apache.org/jira/browse/GROOVY-12000
> Project: Groovy
> Issue Type: Bug
> Components: Static Type Checker
> Affects Versions: 6.0.0-alpha-1
> Reporter: Eric Milles
> Priority: Major
>
> Consider the following:
> {code:groovy}
> @groovy.transform.TypeChecked
> void test(Number number) {
> if (!(number instanceof Cloneable || number instanceof Closeable)) {
> number
> } else {
> number
> }
> }
> {code}
> Currently this passes the check for "can invert" and produces a type like
> (Number & (Cloneable | Closeable))" for the else path. Not sure if we're
> ready for that or not.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
