Copilot commented on code in PR #2518:
URL: https://github.com/apache/groovy/pull/2518#discussion_r3194619370


##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -4800,7 +4807,89 @@ public void visitCastExpression(final CastExpression 
expression) {
         }
     }
 
+    /**
+     * Validates JLS §4.9 well-formedness of an intersection cast target,
+     * and for lambda / method-reference / closure operands picks the
+     * SAM-bearing component as the primary functional target so that
+     * parameter inference (and downstream lambda factory generation)
+     * can proceed. Additional components are stored as
+     * {@link StaticTypesMarker#LAMBDA_MARKERS} on both the cast and
+     * source for use by the bytecode writers.
+     */
+    private void validateAndApplyIntersectionCast(final 
IntersectionTypeClassNode target,
+                                                  final CastExpression 
expression,
+                                                  final Expression source) {
+        ClassNode[] components = target.getComponents();
+        int classCount = 0;
+        boolean classNotFirst = false;
+        for (int i = 0, n = components.length; i < n; i += 1) {
+            ClassNode c = components[i];
+            if (isPrimitiveType(c)) {
+                addStaticTypeError("Intersection type components must be 
reference types: " + prettyPrintType(c), expression);
+            }
+            if (!c.isInterface()) {
+                classCount += 1;
+                if (i != 0) classNotFirst = true;
+                if (Modifier.isFinal(c.getModifiers())) {
+                    addStaticTypeError("Intersection type may not include the 
final class " + prettyPrintType(c), expression);
+                }
+            }
+        }
+        if (classCount > 1) {
+            addStaticTypeError("Intersection type may include at most one 
class component: " + prettyPrintType(target), expression);
+        }
+        if (classNotFirst) {
+            addStaticTypeError("Class component of intersection type must come 
first: " + prettyPrintType(target), expression);
+        }
+
+        boolean isFunctionalSource = source instanceof ClosureExpression

Review Comment:
   `validateAndApplyIntersectionCast` intends to handle lambda sources (it 
later checks `source instanceof LambdaExpression`), but `isFunctionalSource` 
currently only covers `ClosureExpression` and `MethodReferenceExpression`. As a 
result, intersection casts on `LambdaExpression` will skip primary/marker 
selection and metadata setup, breaking serializable/marker handling for 
lambdas. Include `LambdaExpression` in the functional-source check (and keep 
the subsequent lambda-specific serializable handling reachable).
   



##########
src/main/java/org/codehaus/groovy/runtime/IntersectionCastSupport.java:
##########
@@ -0,0 +1,103 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.runtime;
+
+import groovy.lang.Closure;
+import groovy.util.ProxyGenerator;
+import org.codehaus.groovy.runtime.typehandling.GroovyCastException;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Runtime support for intersection-type cast and {@code as} coercion
+ * (GROOVY-11998). Compiler-generated bytecode for {@code (A & B) value} and
+ * {@code value as (A & B)} routes through these helpers when the target is
+ * an intersection type and the source is not a native lambda or method
+ * reference (those cases are handled at compile time via
+ * {@code LambdaMetafactory.altMetafactory} markers).
+ *
+ * @since 5.0.0

Review Comment:
   The Javadoc `@since` tag says `5.0.0`, but this feature is introduced as 
part of Groovy 6.0 (the specs and tests in this PR also say “Since Groovy 
6.0”). Please update the `@since` version to match the actual release version 
that will contain this API.
   



##########
src/main/java/org/codehaus/groovy/ast/IntersectionTypeClassNode.java:
##########
@@ -0,0 +1,103 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.ast;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.StringJoiner;
+
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+
+/**
+ * Represents a user-written intersection type used as the target of a cast
+ * expression or {@code as} coercion, e.g.
+ * <pre>
+ *     (Runnable &amp; Serializable) () -&gt; ...
+ *     value as (A &amp; B)
+ * </pre>
+ *
+ * <p>Distinct from the implicit lowest-upper-bound nodes that
+ * {@link org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor}
+ * synthesises during inference: an instance of this class records the ordered
+ * list of components exactly as written by the user. That ordering is needed
+ * for cast-conversion checks, error messages and (in later phases) bytecode
+ * generation via {@code LambdaMetafactory.altMetafactory} markers.
+ *
+ * <p>Lifecycle: at parse time the components have not yet been resolved to
+ * bound {@link ClassNode}s, so the constructor places all components in the
+ * inherited {@link #getInterfaces() interfaces} array with {@code Object} as
+ * the placeholder superclass. After {@code ResolveVisitor} resolves each
+ * component it should call {@link #reclassifyComponents()} so that the
+ * interfaces array contains only interface components and the superclass is
+ * the (at most one) class component.
+ *
+ * @since 5.0.0

Review Comment:
   The Javadoc `@since` tag is `5.0.0`, but this class is being introduced for 
Groovy 6.0 intersection-type casts. Please align `@since` with the actual 
Groovy version this API ships in.
   



##########
src/spec/doc/core-semantics.adoc:
##########
@@ -759,6 +759,65 @@ The type of the exception depends on the call itself:
 * `MissingMethodException` if the arguments of the call do not match those 
from the interface/class
 * `UnsupportedOperationException` if the arguments of the call match one of 
the overloaded methods of the interface/class
 
+[[intersection-cast]]
+=== Intersection-type cast and coercion
+
+Since Groovy 6.0, the cast and `as` operators accept _intersection types_ — a
+class component (at most one) and any number of interface components joined by
+`&`, mirroring Java's
+https://docs.oracle.com/javase/specs/jls/se21/html/jls-15.html#jls-15.16[JLS 
§15.16]
+intersection cast. The most common use is to opt a lambda or method reference
+into `Serializable`:
+
+[source,groovy]
+----
+include::../test/CoercionTest.groovy[tags=intersection_cast_lambda,indent=0]
+----
+
+For statically compiled lambdas and method references, the additional
+interfaces are threaded through `LambdaMetafactory.altMetafactory` via
+`FLAG_MARKERS` / `FLAG_SERIALIZABLE` — there is no runtime proxy and the
+result implements every component natively, so it can be serialised and
+restored:
+
+[source,groovy]
+----
+include::../test/CoercionTest.groovy[tags=intersection_cast_serializable_roundtrip,indent=0]
+----
+
+The same syntax works in the `as` form, with parentheses around the 
intersection:
+
+[source,groovy]
+----
+include::../test/CoercionTest.groovy[tags=intersection_as_coercion_marker,indent=0]
+----
+
+For closure literals and maps, `as` builds a
+{@link groovy.util.ProxyGenerator} aggregate that implements every interface

Review Comment:
   `{@link ...}` is Javadoc syntax and doesn’t render as a link in AsciiDoc. 
Use the docs’ link conventions instead (e.g., backticks around the type name, 
or an AsciiDoc `link:`/`xref:`), so the reference to 
`groovy.util.ProxyGenerator` renders correctly.
   



##########
src/test/groovy/org/apache/groovy/parser/antlr4/IntersectionCastParserTest.groovy:
##########
@@ -0,0 +1,173 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.parser.antlr4
+
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.IntersectionTypeClassNode
+import org.codehaus.groovy.ast.ModuleNode
+import org.codehaus.groovy.ast.expr.CastExpression
+import org.codehaus.groovy.ast.expr.ClassExpression
+import org.codehaus.groovy.ast.expr.DeclarationExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.BinaryExpression
+import org.codehaus.groovy.ast.stmt.BlockStatement
+import org.codehaus.groovy.ast.stmt.ExpressionStatement
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.control.ParserPlugin
+import org.codehaus.groovy.control.ParserPluginFactory
+import org.junit.jupiter.api.Test
+
+import static org.junit.jupiter.api.Assertions.assertNotNull
+import static org.junit.jupiter.api.Assertions.assertTrue
+import static org.junit.jupiter.api.Assertions.assertEquals
+import static org.junit.jupiter.api.Assertions.assertThrows

Review Comment:
   Unused import: `assertThrows` is imported but not used in this test class.
   



##########
src/main/java/org/codehaus/groovy/classgen/asm/sc/AbstractFunctionalInterfaceWriter.java:
##########
@@ -78,11 +78,32 @@ default void writeFunctionalInterfaceIndy(final 
MethodVisitor methodVisitor,
                                               final String 
samMethodDescriptor, final int implMethodKind,
                                               final ClassNode implClassNode, 
final MethodNode implMethodNode,
                                               final Parameter[] 
implMethodParameters, final boolean serializable) {
+        // GROOVY-11998: delegate to marker-aware overload with no extra 
interfaces
+        writeFunctionalInterfaceIndy(methodVisitor, samMethodName, 
invokedTypeDescriptor,
+                samMethodDescriptor, implMethodKind, implClassNode, 
implMethodNode,
+                implMethodParameters, serializable, ClassNode.EMPTY_ARRAY);
+    }
+
+    /**
+     * Marker-aware variant for intersection-cast targets such as
+     * {@code (Runnable & Cloneable) () -> ...}. Markers are threaded through
+     * {@code LambdaMetafactory.altMetafactory} via {@code FLAG_MARKERS} so the
+     * generated lambda implements every component interface at runtime.
+     *
+     * @since 5.0.0
+     */
+    default void writeFunctionalInterfaceIndy(final MethodVisitor 
methodVisitor,
+                                              final String samMethodName, 
final String invokedTypeDescriptor,
+                                              final String 
samMethodDescriptor, final int implMethodKind,
+                                              final ClassNode implClassNode, 
final MethodNode implMethodNode,
+                                              final Parameter[] 
implMethodParameters, final boolean serializable,
+                                              final ClassNode[] markers) {
+        boolean useAlt = serializable || (markers != null && markers.length > 
0);

Review Comment:
   The new marker-aware overload is documented as `@since 5.0.0`, but it 
appears to be introduced by this Groovy 6.0 intersection-cast work. Please 
update `@since` to the correct version to avoid misleading API documentation.



##########
src/test/groovy/org/apache/groovy/parser/antlr4/IntersectionCastParserTest.groovy:
##########
@@ -0,0 +1,173 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.parser.antlr4
+
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.IntersectionTypeClassNode
+import org.codehaus.groovy.ast.ModuleNode
+import org.codehaus.groovy.ast.expr.CastExpression
+import org.codehaus.groovy.ast.expr.ClassExpression

Review Comment:
   Unused import: `org.codehaus.groovy.ast.expr.ClassExpression` isn’t 
referenced in this test file. Removing unused imports helps keep the test 
focused and avoids confusion when refactoring.
   



##########
src/test/groovy/groovy/transform/stc/IntersectionCastSTCTest.groovy:
##########
@@ -0,0 +1,243 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.transform.stc
+
+import groovy.transform.TypeChecked
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.IntersectionTypeClassNode
+import org.codehaus.groovy.ast.expr.CastExpression
+import org.codehaus.groovy.ast.expr.LambdaExpression
+import org.codehaus.groovy.ast.tools.GenericsUtils
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.codehaus.groovy.control.Phases
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
+import org.codehaus.groovy.control.customizers.ImportCustomizer
+import org.codehaus.groovy.control.messages.SyntaxErrorMessage
+import org.codehaus.groovy.transform.stc.StaticTypesMarker
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.Test
+
+/**
+ * Tests for static type checking of intersection-cast targets (GROOVY-11998 
PR2).
+ *
+ * These tests stop compilation at {@link Phases#SEMANTIC_ANALYSIS} so they
+ * exercise resolution and STC without relying on bytecode generation, which
+ * is delivered in subsequent phases.

Review Comment:
   The class-level comment says compilation stops at 
`Phases.SEMANTIC_ANALYSIS`, but `compileToSemantic` actually compiles to 
`Phases.INSTRUCTION_SELECTION`. Either adjust the phase used (if the intent is 
to stop at semantic analysis) or update the comment/method name to reflect what 
the test is really doing.
   



-- 
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]

Reply via email to