codeconsole commented on PR #3098:
URL: https://github.com/apache/groovy/pull/3098#issuecomment-6527469623
@daniellansun
1. CI: the push of these commits starts a new run.
2. Coverage. Codecov's per-line data for b0aa14e21d: 34 of the 53 lines are
this PR's; the other 19 are #3097's, which Codecov counts here because the PR
is compared with master. 62754d72c9 adds tests for the branches that can be
reached, and 45f47d8a27 removes the ones that cannot. Line numbers are from
b0aa14e21d.
New tests in `StaticCompileRuntimePackageTest` use a fixture with the
public `Open`, the package-private `Shut`, `Shut2 extends Shut`, `Plain` and
the interface `Secret`, and the public `Pub extends Shut`.
`testPackagePrivateClassesOfAnotherClassLoaderAreUsedThroughPublicClassesAsInDynamicCode`
checks that dynamic and `@CompileStatic` code give the same results, and
`testPackagePrivateClassesOfAnotherClassLoaderAreNotNamedInBytecodeOfUsesThroughPublicClasses`
that no instruction names `p/Shut`, `p/Plain` or `p/Secret`:
- `AsmClassGenerator.getDirectlyAccessibleSuperClass` 1387/1388/1390:
`new Shut2()`, whose superclass `Shut` is hidden too, steps to `Open`;
`Open.secret()` returns the interface `Secret`, which has no superclass, so
`Object`.
- `StaticInvocationWriter.findDirectlyAccessibleDeclaration` 273/275:
through `Shut2` the lookup passes the hidden `Shut`; `s.prot()` finds the
protected `Open.prot()`, so the call is made at run time. 442/445/446:
`s.take(s)` finds the public `Open.take(Shut)`, whose parameter type is hidden,
so at run time; `s.take('x')` stays `INVOKEVIRTUAL p/Open.take`. 773:
`list*.@pf`.
- `StaticTypesCallSiteWriter` 555/920: `Shut.SF` and `Shut.@SF = 'w'`
give `GETSTATIC`/`PUTSTATIC p/Open.SF`; 911: `s.@pf = 'q'` gives `PUTFIELD
p/Open.pf`; 899/900: `s.@peer = s` sets a field typed `Shut`, so at run time.
- `StaticTypesMethodReferenceExpressionWriter` 159/162/166/167/165/168:
`Open::toString` as `Function<Shut, String>`, `Open::make` as `Supplier<Shut>`
and `Supplier<Object>`, `Open::take` with the `take(Shut)` overload, and
`Pub::own`, which the hidden `Shut` declares.
- The `!method.isStatic()` side of 275 needs a Java class, since Groovy
rejects an instance method named like a static interface method: in
`testPackagePrivateJavaClassOfAnotherClassLoaderWithInstanceMethodNamedAsStaticMethodOfInterface`,
`Q implements Function` declares an instance `identity()`; without the check,
`q.identity()` compiles to the static `Function.identity()`.
Removed in 45f47d8a27:
- `StaticInvocationWriter.writeInvokeConstructor` 141: the
`DYNAMIC_RESOLUTION` branch and the marker `ConstructorCallTransformer` set for
it. The map-style call already reaches the
`!isClassDirectlyAccessible(call.getType())` branch below, since its
`DIRECT_METHOD_CALL_TARGET` is the generated map constructor;
`testMapStyleConstructionOfPackagePrivateClassOfAnotherClassLoader` still
passes.
- 241: `BytecodeHelper.doCast` already skips `Object`. 254: no caller
passes `null`.
- 490: `written` is always true there, and `getStackLength() > 0` tested
the wrong thing, since the stack can hold other operands; it now compares the
depth before and after the call, which also covers an elided `void` result such
as `h.run()`.
- `StaticTypesCallSiteWriter.makeGetField` 601: the boxed type of a safe
primitive read goes into `valueType`, so the ternary is gone; a new `s?.n`
reads a public `int` field.
- `MethodPointerExpressionWriter` 87 and
`StaticTypesMethodReferenceExpressionWriter.inferredType` 177–182: the
fallbacks to the accessed variable's inferred or origin type never found a type
(178 and 182 never ran in CI); a receiver of a hidden class always carries
`INFERRED_TYPE` itself.
- `StaticTypesTypeChooser` 58: `getTarget` of a `VariableExpression` is
always a `Variable`.
Partial on purpose:
- `StaticTypesTypeChooser.resolveType` 43: the `type != null && current
!= null` checks stay (a44e7610b2). No caller in the compiler passes `null`, but
the method is public and a type checking extension can call it.
- `AsmClassGenerator` 1317: the missing branch is the existing `field ==
null` guard of the public `isFieldDirectlyAccessible`; all three callers pass a
field. The new `isClassDirectlyAccessible` disjunct is covered both ways:
without it, 4 tests fail.
- `StaticTypesCallSiteWriter` 258: `isFieldOfAnotherRuntimePackage` runs
only after `makeGetField` refused a direct read, which never happens for a
public field of a class the bytecode can name or for a field of the same
run-time package.
With JaCoCo over the `sc`, `stc` and `classgen.asm` test packages (4,808
tests), every line this PR adds is covered apart from those. The three new
tests fail without the change (`IllegalAccessError` for `p.Shut2` and for the
Java `p.Q`, and a `CHECKCAST p/Shut`). The Checkstyle and CodeNarc gates pass.
--
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]
