codeconsole commented on PR #3089:
URL: https://github.com/apache/groovy/pull/3089#issuecomment-6526950796
@daniellansun
1. Merged current `master` into the branch (7bee983391), which includes
#3094 / GROOVY-12565, so the coverage job now runs with the
`CopyOnWriteArrayList` fix.
2. When every argument for `T` is a closure, none of them is a witness, so
for an unbounded `<T>` the call binds `T` to no interface and the closures are
passed as they are, as in dynamic Groovy; return type inference is unchanged
(`T` is `Closure`). When the bound of `T` is a functional interface, the
declared parameter is already a SAM, the closures stay witnesses and the
erasure cast converts them, as before. An explicit type argument
(`C.<Supplier<String>>pick({ 'a' }, { 'b' })`) still binds `T`.
`ClosuresSTCTest.testClosuresPassedForTheSameTypeParameter` covers `<T> void
both(T, T)`, `<T> T pick(T, T)` and `<T extends Function<String, String>> T
second(T, T)` with two closures, under `@TypeChecked` and `@CompileStatic`
(ca144e6318).
Checking nested inference found a bug: `list.add(pick({ 'a' }, { 'b' }))`
on a `List<Supplier<String>>` stored the closure. The type checker recorded
`Supplier`, but `StaticInvocationWriter.visitArgument` took the argument type
from the operand stack, where the call of `pick` leaves `Object`. It now takes
the type checker's type, as the variadic path already did (f6f55157f6).
`ClosuresStaticCompileTest.testClosureReturnedForATypeParameterIsConvertedToTheInterfaceTheCallBindsItTo`
covers `add`, `<<`, `Collections.addAll` and `Map.put`, and fails without the
change.
3. No. `getFunctionalTypeArgument` requires `isCompatibleWith(type,
parameterType.redirect())`, the same check `OperandStack.doConvertAndCast`
makes before it emits anything, so after the first cast the operand is the SAM
and the cast to the parameter type emits nothing. The placeholder's redirect is
its first bound, which is also the JVM erasure, so the descriptor matches.
`ClosuresStaticCompileTest.testClosurePassedForATypeParameterWithSeveralBounds`
passes a closure for `<T extends Runnable & Serializable>` bound to a
`SerializableRunnable` subinterface (converted, verifies and runs) and for `<T
extends Comparable<T>>` (converted to `Comparable` by the erasure cast, as
before) (ca144e6318).
4. Either order. The only textual conflict was that both PRs appended a test
at the end of `ClosuresStaticCompileTest`; #3090 now adds its test earlier in
the file, and `git merge-tree` merges the two branches cleanly both ways, to
the same tree. Neither depends on the other's tests: on a merge of both, the
`groovy.transform.stc` and `org.codehaus.groovy.classgen.asm.sc` packages pass.
The STC metadata is on different nodes: #3090 records PARAMETER_TYPE on a named
constructor argument's key, this PR on call arguments; and #3090 turns a
closure assigned to a property into a cast before the setter call is written,
so this PR's writer sees an argument of the interface type and does not convert
it again.
The `groovy.transform.stc` and `org.codehaus.groovy.classgen.asm.sc`
packages pass (4,552 tests), as do the Checkstyle and CodeNarc gates.
--
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]
