[
https://issues.apache.org/jira/browse/GROOVY-12514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135309#comment-18135309
]
ASF GitHub Bot commented on GROOVY-12514:
-----------------------------------------
codeconsole commented on PR #3049:
URL: https://github.com/apache/groovy/pull/3049#issuecomment-6528080927
@daniellansun thanks. New commits on top of cdb685d525, after merging master:
1. **COMPATIBILITY.** 1b7a08c88a adds "Groovy 7 — `N.super.name` and
`Outer.super.name` reach the member of the superclass (GROOVY-12514,
GROOVY-12517)". Each claim was checked against master with scripts: in dynamic
Groovy, `N.super.p`/`.f` and `Outer.super.p`/`.f` read and wrote the instance's
own member, as did the `.@` forms, and with an enclosing instance of a subclass
of `Outer` they reached the subclass's member; `N.super.m()` and
`Outer.super.m()` already called the superclass's method; under `@TypeChecked`
and `@CompileStatic` all the property and field forms were "No such property:
super" compile errors.
2. **`Outer.super.@field`.** Fixed in 585caf6f1b:
`AsmClassGenerator.visitAttributeExpression` picks `get/setFieldOnSuper` with
`isSuperOrOuterSuper`, as `visitPropertyExpression` does. `N.super.@x` was
already right on this branch, since `ResolveVisitor` rewrites `N.super` to
`super` (master read `N`'s own field). Under SC `Outer.super.@x` was already a
direct `GETFIELD`/`PUTFIELD` of the superclass's field, because the type
checker types `Outer.super` as `Outer`'s superclass; with the dynamic fix it
would have gone through `getFieldOnSuper`, so
`StaticTypesCallSiteWriter.fallbackAttributeOrPropertySite` now keeps it a
direct field access, next to `makeSuperPropertySite`.
Testing it found one more case: `InvocationWriter.makeCall` passed the
nested class as the sender, so dynamic `Outer.super` was resolved against the
class of the enclosing instance at run time; with an instance of `Sub extends
Outer`, `Outer.super.q` read `Outer`'s `q` and `Outer.super.m()` called
`Outer`'s `m` (the latter on master too). 2fd3cbedc3 passes `Outer` as the
sender.
`InnerClassTest.testQualifiedSuperField` (read, write, `++`, `+=`,
closure and lambda for `Outer.super.@x` and `N.super.@x`, with same-named
fields in the superclass and the subclass, all three modes) and
`testQualifiedSuperOfOuterClassWithInstanceOfSubclass` fail without their
changes in dynamic and `@TypeChecked` code;
`FieldsAndPropertiesStaticCompileTest.testOuterSuperFieldIsAccessedDirectly`
checks the SC bytecode and fails without the SC part.
3. **#3071.** `git merge-tree` with #3071's head reports no conflicting
files, and the merge has the order you describe: the interface check returns
early in `checkThisAndSuperAsPropertyAccess`, the `N.super` rewrite runs after
it only for a non-interface nested class, and in `existsProperty` the
`Outer.super` branch follows the interface branch and requires a non-interface
type. On the merge,
`Groovy12526.testEnclosingClassSuperIsOutsideTheInterfaceCheck` fails: it
expects `D.super.value()` from `D.B` to be "No such property: super", which is
what GROOVY-12517 removes. Whichever is merged second should make it an
`assertScript` returning `'C'`; the description lists it. `InnerClassTest`,
`ThisAndSuperTest` and `Groovy12526pt2` pass on the merge.
4. **Shared helper.** da0bf45b22 adds
`ExpressionUtils.isOuterSuperExpression` (`Type.super` with `Type` not an
interface), used by `AsmClassGenerator.isSuperOrOuterSuper`,
`InvocationWriter.writeInvokeMethod` (replacing `isOuterClassSuper`),
`StaticTypeCheckingVisitor.checkSuperCallFromClosure` and
`StaticTypesCallSiteWriter.findSuperPropertyExtension`, which had the check
without the interface guard. `InvocationWriter.isClassWithSuper` and
`StaticInvocationWriter.isClassWithSuper` stay as they are, since they match
interfaces on purpose (`INVOKESPECIAL` for `Iface.super.m()`).
5. **Cherry-pick.** All code and test commits cherry-pick cleanly onto
GROOVY_6_0_X, and `InnerClassTest`, `ClosuresSTCTest`,
`FieldsAndPropertiesSTCTest` and `FieldsAndPropertiesStaticCompileTest` pass
there. The COMPATIBILITY.md commit conflicts only because 6.0.x has no Groovy 7
sections; there it goes after the 6.0.1 entries, as for GROOVY-12521.
**`StaticTypesCallSiteWriter:900`, the getter from
`DIRECT_METHOD_CALL_TARGET`:** not done. For `++` and compound assignments the
target on that expression is the setter, so the getter is looked up by name
anyway. And where the two can differ, the metadata is the odd one out:
`PropertyNode.getGetterNameOrDefault` prefers a declared `getX` to `isX`, while
plain `super.f`, dynamic Groovy and `findSuperPropertyAccessor` pick `isX`.
Taking the metadata would make `{ -> super.f }` call `getF` while `super.f`
calls `isF`; that disagreement belongs in the type checker's choice of getter
name.
`gls`, `groovy.transform.stc`, `org.codehaus.groovy.classgen.asm.sc` and
`groovy.ThisAndSuperTest` pass (5,535 tests), as do the Checkstyle and CodeNarc
gates.
> STC: qualified this or super naming the current nested class (N.this, N.super
> inside N) is rejected
> ---------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12514
> URL: https://issues.apache.org/jira/browse/GROOVY-12514
> Project: Groovy
> Issue Type: Bug
> Components: Compiler, Static Type Checker
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> {{N.this}} inside a nested class {{N}} is {{this}}, and {{N.super.m()}} is
> {{super.m()}}. The type checker rejects both:
> {code:groovy}
> class Base { String m() { 'base' } }
> @groovy.transform.CompileStatic
> class Box {
> static class N extends Base {
> String id = 'n'
> String m() { 'n' }
> String test() { N.this.id + ' ' + N.super.m() }
> }
> }
> new Box.N().test()
> {code}
> {code}
> [Static type checking] - No such property: this for Class or static property
> for class: Box$N
> [Static type checking] - No such property: super for Class or static property
> for class: Box$N
> {code}
> (plus the errors that follow from them for {{id}} and {{m()}}). Dynamic
> Groovy returns {{n base}}, and javac accepts the same code. The same errors
> are reported under {{@TypeChecked}}, for an inner (non-static) class {{N}},
> for {{N.this.pm()}} and {{N.this.id = 'w'}}, for {{N.super.p}}, and inside a
> closure or lambda in {{N}}. {{N.this}} inside an anonymous class created in
> {{N}} already works, because {{N}} is an outer class there. Seen in 5.0.8,
> 6.0.0 and master.
> In a top-level class, {{Top.this}} and {{Top.super}} are rejected in every
> mode ("The usage of 'Class.this' and 'Class.super' is only allowed in
> nested/inner classes."), as GROOVY-4321 decided and {{StaticThisTest}} pins,
> so they are not part of this issue.
> Expected: under {{@TypeChecked}} and {{@CompileStatic}}, {{N.this}} and
> {{N.super}} inside the nested class {{N}} mean {{this}} and {{super}}, as in
> dynamic Groovy, except in a static context, where they stay an error.
> PR: https://github.com/apache/groovy/pull/3049
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
