[
https://issues.apache.org/jira/browse/GROOVY-12727?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140590#comment-18140590
]
ASF GitHub Bot commented on GROOVY-12727:
-----------------------------------------
daniellansun commented on PR #3265:
URL: https://github.com/apache/groovy/pull/3265#issuecomment-6718282980
REQUEST_CHANGES
The Pekko assignment and return both type-check, for a lambda and for a
closure, and GenericsSTCTest is green including the three new methods. The
witness is written onto StaticMethodCallExpression and then read only from the
enclosing-call stack, which visitStaticMethodCallExpression never pushes. By
INSTRUCTION_SELECTION, StaticImportVisitor has already turned a same-class
static call and an `import static` call into StaticMethodCallExpression, so
those still fail with the original error:
static Beh<String> go(int max) { setup(c -> new Bot(c, max)) }
// Cannot call Bot#<init>(Ctx<String>, int) with arguments [Ctx<?
extends Object>, int]
javac accepts that. `import static java.util.Comparator.comparing` then
`Comparator<String> c = comparing(s -> s.length())` fails here with
Object#length() and Comparator<Object> assigned to Comparator<String>.
Qualified Behaviors.setup(...) stays a MethodCallExpression, which is the only
shape the new tests hit.
Please push/pop the static call in visitStaticMethodCallExpression the same
way visitMethodCallExpression does, and add those two calls beside
testReturnTypeInferenceWithClosure5. makeArgumentList returns the same
ArgumentListExpression the parser stored, so the identity check in
extractGenericsConnectionsFromCallTarget will then succeed.
A few targets the helper’s comment says it determines are still unresolved.
I compared them with javac on this tip:
- `<T extends CharSequence> List<T> ids(Function<T,T>)` assigned to
`List<String>` leaves the lambda parameter as CharSequence
(`CharSequence#substring` is missing; the assignment is List<CharSequence> to
List<String>). javac uses T = String. An out-of-bound target (`T extends
Number` against a call whose parameter needs String) stays at Number, so this
is under-acceptance, not a bad instantiation. putIfAbsent still does not check
the witness against T’s upper bounds.
- `List<List<T>>` assigned to `List<List<String>>`, and `Map<A,B>` assigned
to `Map<String,Integer>`, both leave the variables at Object
(`Object#length()`, List<List<Object>>, Map<Object,Object>). The return type
still carries placeholders named T, A, and B. ArrayList<T> versus List<String>
works; the nested and two-variable cases do not. Map<T,T> versus
Map<String,Integer> is rejected, which matches javac, but only as a last-write
in storeGenericsConnection rather than a conflict error.
- `Beh<? super String> b = Behaviors.setup(c -> new Bot(c))` is still Ctx<?
extends Object>. javac accepts it: ? super String pins T to String, and
Beh<String> is a subtype of Beh<? super String>. Skipping every wildcard is
right for Beh<? extends Object> (test 7, and javac agrees) and too coarse for a
concrete ? super or ? extends bound.
- A ternary in an assignment, a call passed to take(Beh<String>), and
Optional.of(setup(...)) still show Ctx<? extends Object>. The target is applied
to the ternary node and not to the branch, and checkForTargetType still has the
TODO for argument parameters. A cast does infer T, because visitCastExpression
calls applyTargetType; javac does not treat a cast as a poly context for the
invocation (JLS 15.12) and rejects `(Beh<String>) setup(c -> new Bot(c))`. The
Groovy result is sound. Worth a comment so the cast hook is intentional.
- inferReturnTypeGenerics does not see CALL_TARGET_TYPE. `List<CharSequence>
xs = cons(s -> 'x')` for `Function<? super T, ? extends T>` is inferred as
List<String> and rejected. javac uses T = CharSequence.
GROOVY-11446 still type-checks (`List<Number> x = m(d)` for `List<?> d` and
`<T> List<T> m(List<? extends T>)`). There is no SAM argument, so this patch
does not touch it. javac rejects it. The lambda form of that pattern is
rejected here. Not a regression; not fixed either.
I would keep Closure7 as the unbounded-wildcard rejection, and add:
same-class static and import static; T extends CharSequence against
List<String>; List<List<T>> and Map<A,B>; Beh<? super String> must pass; the
Map<T,T> / Map<String,Integer> conflict must fail; one ternary and one argument
context. String as a target is a weak assertion on its own, because STC will
accept almost any value as a String. The Bot constructor in Closure5 is the
right kind of check.
The ReturnAdder widening looks safe: the listener constructor does not
rewrite the method, and an explicit return is stamped only from
visitReturnStatement. I do not see a merge conflict with the PECS work on the
lines this patch adds; that branch is not an ancestor of 60711c748721. Once a ?
super bound is stored as a witness, it will meet the tighter
compatibleConnection on that branch and should be rechecked.
> STC: a generic method's type parameter isn't inferred from the return-type
> target when the argument is a lambda
> ---------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12727
> URL: https://issues.apache.org/jira/browse/GROOVY-12727
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> Under {{@TypeChecked}} (and {{@CompileStatic}}), when a generic method's only
> argument is an implicitly typed lambda, the type parameter is not inferred
> from the expected return type. The lambda's parameter is typed with a
> wildcard ({{Ctx<? extends Object>}}), and a call inside the lambda that needs
> the concrete type is rejected. javac infers {{T = String}} from the
> return-type target and compiles the equivalent Java.
> This is the standard Apache Pekko (and Akka) typed-actor factory,
> {{Behaviors.setup(context -> new MyActor(context))}}, so statically compiled
> Pekko code needs a type witness or an explicitly typed lambda parameter.
> h3. Repro (no dependencies)
> {code:groovy}
> import java.util.function.Function
> import groovy.transform.TypeChecked
> class Ctx<T> {}
> class Beh<T> {}
> class Behaviors {
> static <T> Beh<T> setup(Function<Ctx<T>, Beh<T>> factory) {
> factory.apply(new Ctx<T>()) }
> }
> class Bot extends Beh<String> {
> Bot(Ctx<String> c, int max) {}
> }
> @TypeChecked
> class M {
> static Beh<String> lambda(int max) { Behaviors.setup(c -> new Bot(c,
> max)) }
> }
> {code}
> h3. Actual
> {noformat}
> [Static type checking] - Cannot call Bot#<init>(Ctx<java.lang.String>, int)
> with arguments [Ctx<? extends java.lang.Object>, int]
> {noformat}
> h3. Expected
> Compiles, as the Java equivalent does with javac ({{T}} inferred as
> {{String}} from the {{Beh<String>}} return type).
> h3. Workarounds (both compile on 6.0.0)
> {code:groovy}
> Behaviors.<String>setup(c -> new Bot(c, max)) // explicit
> type witness
> Behaviors.setup((Ctx<String> c) -> new Bot(c, max)) // explicitly
> typed lambda parameter
> {code}
> h3. Notes
> * The closure form, Behaviors.setup \{ c -> new Bot(c, max) }, fails with the
> same message.
> * An assignment target fails the same way as a return target: {{Beh<String> b
> = Behaviors.setup(c -> new Bot(c, max))}}.
> * Possibly related, not duplicates:
> ** GROOVY-11446: target-type handling for a generic method with wildcards, in
> the opposite direction (accepted when it should fail).
> ** GROOVY-11077: lambda parameter types from a wildcard SAM; no generic
> method or inference involved.
> ** GROOVY-12714 / PR #3246: javac-compatibility changes to wildcard
> containment in the same visitor; they don't appear to supply this inference
> (not verified against the branch).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
