[
https://issues.apache.org/jira/browse/GROOVY-12514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135243#comment-18135243
]
ASF GitHub Bot commented on GROOVY-12514:
-----------------------------------------
daniellansun commented on code in PR #3049:
URL: https://github.com/apache/groovy/pull/3049#discussion_r4425287548
##########
src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java:
##########
@@ -1498,9 +1508,9 @@ public void visitPropertyExpression(final
PropertyExpression expression) {
if (!visited) {
MethodCallerMultiAdapter adapter;
if (controller.getCompileStack().isLHS()) {
- adapter = isSuperExpression(objectExpression) ?
setPropertyOnSuper : isGroovyObject(expression) ? setGroovyObjectProperty :
setProperty;
+ adapter = isSuperOrOuterSuper(objectExpression) ?
setPropertyOnSuper : isGroovyObject(expression) ? setGroovyObjectProperty :
setProperty;
Review Comment:
Property form now goes through isSuperOrOuterSuper → get/setPropertyOnSuper,
but the attribute adapters in visitAttributeExpression (tip ~1550) still key
only on isSuperExpression. Outer.super.@q will load Outer.this and then
getField without useSuper, i.e. Outer's own field — the same bug shape this PR
fixes for Outer.super.q.
Please mirror isSuperOrOuterSuper there (and cover Outer.super.@x /
N.super.@x in InnerClassTest), or open a follow-up ticket and call it out in
the PR so the @ form is not left silently wrong.
##########
src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java:
##########
@@ -1355,6 +1355,16 @@ public static FieldNode
getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(f
);
}
+ /**
+ * Determines if an expression is "super" or "Outer.super" for a class: a
+ * property of it is a property of the superclass of the instance.
+ */
+ private static boolean isSuperOrOuterSuper(final Expression expression) {
Review Comment:
isSuperOrOuterSuper matches InvocationWriter.isOuterClassSuper /
isClassWithSuper (GROOVY-11611). Fine as a local copy for this fix, but
consider one shared ExpressionUtils (or package) helper so the !isInterface()
guard stays aligned with GROOVY-12526 (#3071).
##########
src/main/java/org/codehaus/groovy/control/ResolveVisitor.java:
##########
@@ -1192,6 +1192,11 @@ protected Expression transformPropertyExpression(final
PropertyExpression pe) {
}
checkThisAndSuperAsPropertyAccess(xe);
+ if (isSuperOfCurrentNestedClass(xe)) { // "Type.super" within nested
class Type is "super"
Review Comment:
Good place for the rewrite (after checkThisAndSuperAsPropertyAccess). When
rebasing onto or against #3071 (GROOVY-12526), keep the interface-super
rejection as an early path for ClassExpression + interface + "super", and keep
this rewrite only for non-interface nested classes — the interface early-return
must not swallow N.super.
##########
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java:
##########
@@ -828,6 +841,103 @@ && getField(receiverType, name, FieldNode::isPublic) ==
null) {
super.fallbackAttributeOrPropertySite(expression, objectExpression,
name, adapter);
}
+ /**
+ * Reads or writes "super.name", as the operand of "++" or "--" or the
target
+ * of a compound assignment, through the accessor of the superclass.
Within a
+ * closure or lambda, and for "Outer.super.name", the accessor is called
through
+ * the super$ helper the type checker asked the enclosing class for. An
accessor
+ * that is an extension method is called directly, as it needs no helper.
+ */
+ private boolean makeSuperPropertySite(final PropertyExpression expression,
final Expression objectExpression, final String name, final boolean lhs) {
+ ClassNode helperType =
expression.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
+ MethodNode target = findSuperPropertyExtension(expression,
objectExpression, name, lhs);
+ Expression receiver;
+ if (target != null) {
+ helperType = null;
+ receiver = objectExpression;
+ } else if (helperType != null) {
+ MethodNode accessor = findSuperPropertyAccessor(expression,
helperType.getSuperClass(), name, lhs);
+ List<MethodNode> helpers =
helperType.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
+ if (accessor == null || helpers == null ||
helpers.stream().noneMatch(mn -> isSameSuperMopMethod(mn, accessor))) {
+ return false;
+ }
+ target = StaticCompilationMopWriter.getSuperMopMethod(helperType,
accessor);
+ receiver = varX("this");
+ } else if (isSuperExpression(objectExpression) &&
!controller.isInGeneratedFunction()) {
+ target = findSuperPropertyAccessor(expression,
controller.getClassNode().getSuperClass(), name, lhs);
+ if (target == null) {
+ return false;
+ }
+ receiver = objectExpression;
+ } else {
+ return false;
+ }
+
+ CompileStack compileStack = controller.getCompileStack();
+ OperandStack operandStack = controller.getOperandStack();
+ MethodCallExpression call;
+ int value = 0;
+ if (lhs) { // the value to store is on the operand stack
+ ClassNode valueType = operandStack.getTopOperand();
+ value = compileStack.defineTemporaryVariable("$value", valueType,
true);
+ call = callX(receiver, target.getName(), args(new
VariableSlotLoader(valueType, value, operandStack)));
+ } else {
+ call = callX(receiver, target.getName());
+ }
+ call.setImplicitThis(helperType != null);
+ call.setMethodTarget(target);
+ call.setSourcePosition(expression);
+ compileStack.pushLHS(false);
+ call.visit(controller.getAcg());
+ compileStack.popLHS();
+ if (lhs) {
+ operandStack.pop();
+ compileStack.removeVar(value);
+ }
+ return true;
+ }
+
+ private static MethodNode findSuperPropertyAccessor(final
PropertyExpression expression, final ClassNode superType, final String name,
final boolean setter) {
Review Comment:
findSuperPropertyAccessor looks up isX then getX on the superclass, while
the setter path trusts DIRECT_METHOD_CALL_TARGET. That matches
Verifier-generated boolean properties and the new tests. Optional hardening:
for getters, prefer DIRECT_METHOD_CALL_TARGET when present (as setters do) so a
synthetic isX recorded on SUPER_MOP_METHOD_REQUIRED cannot disagree with a lone
getX on the ClassNode.
> 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)
