This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11818 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 403117a1894c88396cdbeb6d3b21a69a004ca465 Author: Eric Milles <[email protected]> AuthorDate: Mon Dec 29 19:21:45 2025 -0600 GROOVY-11818: AIC variable capture --- .../groovy/classgen/AsmClassGenerator.java | 2 +- .../groovy/classgen/InnerClassVisitor.java | 30 +++++------ src/test/groovy/bugs/Groovy9120.groovy | 60 ---------------------- .../groovy/gls/innerClass/InnerClassTest.groovy | 37 ++++++++++++- .../classgen/asm/AbstractBytecodeTestCase.groovy | 26 +++++----- 5 files changed, 65 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 3869830267..99d786c0a8 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -1240,7 +1240,7 @@ public class AsmClassGenerator extends ClassGenerator { // GROOVY-8448: "this.name" from anon. inner class if (fieldNode != null && !expression.isImplicitThis() && (fieldNode.getModifiers() & ACC_SYNTHETIC) != 0 - && fieldNode.getType().equals(ClassHelper.REFERENCE_TYPE)) { + /*&& fieldNode.getType().equals(ClassHelper.REFERENCE_TYPE)*/) { fieldNode = null; } // GROOVY-10695: "this.name" or "Type.name" where "name" is non-static diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java index 0281bfc0a9..19d1b50589 100644 --- a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java +++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java @@ -52,7 +52,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; import static org.codehaus.groovy.transform.trait.Traits.isTrait; import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_MANDATED; -import static org.objectweb.asm.Opcodes.ACC_PUBLIC; +import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_STATIC; import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC; @@ -204,26 +204,26 @@ public class InnerClassVisitor extends InnerClassVisitorHelper { addFieldInit(thisParameter, thisField, block); } - // for each shared variable, add a Reference field - for (Iterator<Variable> it = scope.getReferencedLocalVariablesIterator(); it.hasNext();) { - Variable var = it.next(); - - VariableExpression ve = new VariableExpression(var); - ve.setClosureSharedVariable(true); - ve.setUseReferenceDirectly(true); + // for each shared variable, add a field + for (Iterator<Variable> it = scope.getReferencedLocalVariablesIterator(); it.hasNext(); ) { Variable var = it.next(); + var ve = new VariableExpression(var); +// ve.setClosureSharedVariable(true); +// ve.setUseReferenceDirectly(true); expressions.add(pCount, ve); - ClassNode referenceType = ClassHelper.REFERENCE_TYPE.getPlainNodeReference(); - Parameter p = new Parameter(referenceType, "p" + pCount); +// ClassNode referenceType = ClassHelper.REFERENCE_TYPE.getPlainNodeReference(); +// Parameter p = new Parameter(referenceType, "p" + pCount); + var p = new Parameter(var.getType(), "p" + pCount); p.setOriginType(var.getOriginType()); parameters.add(pCount++, p); - VariableExpression initial = new VariableExpression(p); + var initial = new VariableExpression(p); initial.setSynthetic(true); - initial.setUseReferenceDirectly(true); - FieldNode pField = innerClass.addFieldFirst(ve.getName(), ACC_PUBLIC | ACC_SYNTHETIC, referenceType, initial); - pField.setHolder(true); - pField.setOriginType(ClassHelper.getWrapper(var.getOriginType())); +// initial.setUseReferenceDirectly(true); + FieldNode pField = innerClass.addFieldFirst(ve.getName(), ACC_PRIVATE | ACC_SYNTHETIC, p.getType(), initial); +// pField.setOriginType(ClassHelper.getWrapper(p.getOriginType())); +// pField.setOriginType(p.getOriginType()); +// pField.setHolder(true); } innerClass.addConstructor(ACC_SYNTHETIC, parameters.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, block); diff --git a/src/test/groovy/bugs/Groovy9120.groovy b/src/test/groovy/bugs/Groovy9120.groovy deleted file mode 100644 index cb6bb99207..0000000000 --- a/src/test/groovy/bugs/Groovy9120.groovy +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package bugs - -import groovy.transform.CompileStatic -import org.junit.Test - -import static groovy.test.GroovyAssert.assertScript - -@CompileStatic -final class Groovy9120 { - - @Test - void testLocalVariableReferencesFromAIC() { - assertScript ''' - import java.util.concurrent.Callable - - interface Face9120 { - Runnable runnable() - Callable<Long> callable() - } - - static Face9120 make() { - final long number = 42 - return new Face9120() { - @Override - Runnable runnable() { - return { -> - assert "number is ${number}" == 'number is 42' - } - } - @Override - Callable<Long> callable() { - return { -> number } - } - } - } - - def face = make() - face.runnable().run() - assert "number is ${face.callable().call()}" == 'number is 42' - ''' - } -} diff --git a/src/test/groovy/gls/innerClass/InnerClassTest.groovy b/src/test/groovy/gls/innerClass/InnerClassTest.groovy index 89ceaedc2d..784d7aa99a 100644 --- a/src/test/groovy/gls/innerClass/InnerClassTest.groovy +++ b/src/test/groovy/gls/innerClass/InnerClassTest.groovy @@ -166,7 +166,7 @@ final class InnerClassTest { } assert a.call() == 1 x = 2 - assert a.call() == 2 + assert a.call() == 1 ''' } @@ -215,6 +215,39 @@ final class InnerClassTest { ''' } + // GROOVY-9120 + @Test + void testAccessLocalVariableFromClosureInAIC2() { + assertScript ''' + import java.util.concurrent.Callable + + interface Face9120 { + Runnable runnable() + Callable<Long> callable() + } + + static Face9120 make() { + final long number = 42 + return new Face9120() { + @Override + Runnable runnable() { + return { -> + assert "number is ${number}" == 'number is 42' + } + } + @Override + Callable<Long> callable() { + return { -> number } + } + } + } + + def face = make() + face.runnable().run() + assert "number is ${face.callable().call()}" == 'number is 42' + ''' + } + // GROOVY-9825 @Test void testAccessSuperInterfaceConstantWithInnerClass() { @@ -1582,7 +1615,7 @@ final class InnerClassTest { def v = false def a = new A() { @Override void m() { - assert v == true + assert !v } } v = true diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/AbstractBytecodeTestCase.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/AbstractBytecodeTestCase.groovy index c21245bbad..4ba128e4d5 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/AbstractBytecodeTestCase.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/AbstractBytecodeTestCase.groovy @@ -48,17 +48,17 @@ abstract class AbstractBytecodeTestCase extends GroovyTestCase { @Override protected void assertScript(final String script) throws Exception { - CompilationUnit unit = null - GroovyShell shell = new GroovyShell(new GroovyClassLoader() { + def array = new CompilationUnit[1] + def shell = new GroovyShell(new GroovyClassLoader() { @Override protected CompilationUnit createCompilationUnit(final CompilerConfiguration config, final CodeSource source) { - unit = super.createCompilationUnit(config, source) + array[0] = super.createCompilationUnit(config, source) } }) try { shell.evaluate(script, testClassName) } finally { - if (unit != null) { + if (array[0] instanceof CompilationUnit unit) { try { def firstClass = unit.classes.find { it.name == unit.firstClassNode.name } sequence = extractSequence(firstClass.bytes, extractionOptions) @@ -114,34 +114,36 @@ abstract class AbstractBytecodeTestCase extends GroovyTestCase { InstructionSequence extractSequence(final byte[] bytes, final Map options = [method: 'run']) { def out = new StringBuilderWriter() - def tcv - tcv = new TraceClassVisitor(new ClassVisitor(CompilerConfiguration.ASM_API_VERSION) { + Reference<TraceClassVisitor> ref = [] + def tcv = new TraceClassVisitor(new ClassVisitor(CompilerConfiguration.ASM_API_VERSION) { + List getText() { ref.p.text } @Override MethodVisitor visitMethod(final int access, final String name, final String desc, final String signature, final String... exceptions) { if (options.method == name) { - // last in "tcv.p.text" is a list that will be filled by "super.visit" - tcv.p.text.add(tcv.p.text.size() - 2, '--BEGIN--\n') + // last in "text" is a list that will be filled by "super.visit" + text.add(text.size() - 2, '--BEGIN--\n') try { super.visitMethod(access, name, desc, signature, exceptions) } finally { - tcv.p.text.add('--END--\n') + text.add('--END--\n') } } } @Override FieldVisitor visitField(final int access, final String name, final String desc, final String signature, final Object value) { if (options.field == name) { - // last in "tcv.p.text" is a list that will be filled by "super.visit" - tcv.p.text.add(tcv.p.text.size() - 2, '--BEGIN--\n') + // last in "text" is a list that will be filled by "super.visit" + text.add(text.size() - 2, '--BEGIN--\n') try { super.visitField(access, name, desc, signature, value) } finally { - tcv.p.text.add('--END--\n') + text.add('--END--\n') } } } }, new PrintWriter(out)) + ref.set(tcv) new ClassReader(bytes).accept(tcv, 0) new InstructionSequence(instructions: out.toString().split('\n')*.trim()) }
