blackdrag commented on code in PR #2709: URL: https://github.com/apache/groovy/pull/2709#discussion_r3646429161
########## src/test/groovy/org/codehaus/groovy/transform/PackedClosureBoundariesTest.groovy: ########## @@ -0,0 +1,129 @@ +/* + * 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 org.codehaus.groovy.transform + +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.Phases +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertEquals + +/** + * The packability boundary as one readable matrix — executable documentation of GEP-27's + * "packability decision procedure". Each case is a method body containing one closure literal; + * the assertion is whether that literal packs (no generated closure class) or declines (keeps + * its class), under the flag with {@code @CompileStatic} or dynamic compilation as noted. + * Individual gates have focused behavioural tests elsewhere; this class exists so the whole + * boundary can be read top-to-bottom in one place. + */ +final class PackedClosureBoundariesTest { + + private static final String PROP = CompilerConfiguration.CLOSURE_PACKING + + /** [description, dynamic-packs?, cs-packs?, method body containing exactly one closure literal] */ + private static final List CASES = [ + // ---- the syntactic no-free-name subset: packs everywhere ------------------------------- + ['no free names', true, true, 'def m(List<Integer> xs) { xs.collect { x -> x + 1 } }'], + ['implicit it', true, true, 'def m(List<Integer> xs) { xs.collect { it * 2 } }'], + ['explicit zero params', true, true, 'def m() { def c = { -> 42 }; c() }'], + ['captured local (read-only)', true, true, 'def m(List<Integer> xs) { def k = 10; xs.collect { x -> x + k } }'], + ['captured local (written)', true, true, 'def m(List<Integer> xs) { int t = 0; xs.each { t += it }; t }'], + ['parameter receiver', true, true, 'def m(List<Integer> xs) { xs.collect { it.toString() } }'], + // ---- free names: the types-or-trust boundary (dynamic declines, CS proof packs) -------- + ['implicit-this method call', false, true, 'def helper(x) { x }\ndef m(List<Integer> xs) { xs.collect { helper(it) } }'], + ['bare field-bound name', false, true, 'int field = 1\ndef m(List<Integer> xs) { xs.collect { it + field } }'], + ['explicit this-property', false, true, 'int field = 1\ndef m(List<Integer> xs) { xs.collect { it + this.field } }'], + // ---- real-Closure semantics: declines everywhere ---------------------------------------- + ['uses delegate', false, false, 'def m() { def c = { delegate.toString() }; c }'], + ['uses owner', false, false, 'def m() { def c = { owner.toString() }; c }'], + ['default parameter values', false, false, 'def m() { def c = { int x = 1 -> x }; c(2) }'], + // ---- escapes: declines everywhere -------------------------------------------------------- + ['returned', false, false, 'Closure m() { return { it } }'], + ['stored to property', false, false, 'def m(Map attrs) { attrs.handler = { it } }'], + ['in a collection literal', false, false, 'def m() { [{ it }] }'], Review Comment: I think the given reason in those 3 last tests for why this is not inlined is incorrect. `returned` should be hoistable. That it is not is due to the lack of default parameter support for it. `stored to property` then actually looks like the exact same case. It does not matter, that `attrs.handler` would be dynamic. And in the case `in a collection literal` you could see the return list as a special case of a returned closure, but again the real reason is the missing default parameter support. If you wanted to imply with `escapes: declines everywhere` that an escaping Closure is never hoisted, then you should actually add a case without it and without default parameters. I guess I am missing an explanation of why an escaping Closure is declined. Especially if I assume non of the other tests here is testing that already ########## src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java: ########## @@ -158,6 +253,1030 @@ public void writeClosure(final ClosureExpression expression) { controller.getOperandStack().replace(ClassHelper.CLOSURE_TYPE, localVariableParams.length); } + /** + * GEP-27 capability analysis: choose the compilation strategy for a closure literal. + * <p> + * A closure is packed (S1, {@link PackStrategy#PACKED_ADAPTER}) when it is <em>triggered</em> — + * either the enclosing scope opts in via {@code @PackedClosures} (the dynamic trust path), or the + * capability analysis <em>proves</em> it delegate-independent under {@code @CompileStatic} (the + * automatic path, see {@link #isDelegateIndependent}) — and it is in a packable position: + * it needs no real {@code Closure} semantics we cannot reproduce ({@link #isPackable}), it is + * written directly in a method rather than nested in another closure (owner-retargeting for + * nested closures is later work), and it does not visibly escape its method. Otherwise it stays a + * full generated class (S0, {@link PackStrategy#FULL_CLASS}) exactly as today. + */ + private PackStrategy chooseStrategy(final ClosureExpression expression) { + // The complete packability decision, in order (GEP-27 "The packability decision procedure"; + // any decline keeps the closure a generated class, exactly as today): + // 1. needs real Closure semantics (owner/delegate/thisObject/resolveStrategy/metaClass/ + // super, default parameter values, anonymous inner class)? + if (!isPackable(expression)) return PackStrategy.FULL_CLASS; + // 2. scope opted out? The most-specific @PackedClosures mode wins (method over class), and + // DISABLED beats an enclosing opt-in AND the flag. + PackMode mode = annotatedMode(); + if (mode == PackMode.DISABLED) return PackStrategy.FULL_CLASS; + boolean annotated = (mode != null); // LENIENT/WARN/STRICT all opt in (DISABLED handled above) + // 3. triggered and sound? annotation (dynamic trust), flag + syntactic no-free-name proof + // (dynamic), or flag/annotation + the type checker's delegate-independence proof (CS, + // which also declines mixed-mode dynamic islands and Map-owner property semantics). + if (!isPackTriggered(expression, annotated)) return PackStrategy.FULL_CLASS; + // 4. structural position: directly in a method of the owner class (nested closures await + // owner-retargeting)? + if (controller.isInGeneratedFunction()) return PackStrategy.FULL_CLASS; + // 5. visibly escapes (field/property/index store, return, <<, collection literal)? + if (escapesEnclosingMethod(expression)) return PackStrategy.FULL_CLASS; + // 6. visibly serialization-bound (Serializable cast/coercion, writeObject directly or via + // a literal-holding local)? + if (serializationBound(expression)) return PackStrategy.FULL_CLASS; + // 7. a field initializer (a field store the escape walk cannot see)? + if (isFieldInitializer(expression)) return PackStrategy.FULL_CLASS; + // 8.-10. a context the adapter cannot inhabit: intersection cast (per-literal marker + // interfaces), trait body ($Trait$Helper's synthetic receiver), or a this(...)/super(...) + // argument (uninitializedThis cannot be the dispatch receiver)? + if (isIntersectionTyped(expression)) return PackStrategy.FULL_CLASS; + if (isInTraitContext()) return PackStrategy.FULL_CLASS; + if (controller.getCompileStack().isInSpecialConstructorCall()) return PackStrategy.FULL_CLASS; + return PackStrategy.PACKED_ADAPTER; + } + + /** + * A closure in a field initializer ({@code def action = { ... }} at class level) is stored + * into a field by definition — the same escape the escape gate declines for in-method stores — + * but field initializers compile outside the enclosing method's visible code, so the escape + * walk cannot see them. Detected directly against the class's fields (identity match). + */ + private boolean isFieldInitializer(final ClosureExpression expression) { + for (FieldNode fn : controller.getClassNode().getFields()) { + if (fn.getInitialValueExpression() == expression) return true; + } + return false; + } + + /** + * A closure literal cast to an intersection type (e.g. {@code (Runnable & Serializable) { }}) + * needs its <em>generated class</em> to declare the marker interfaces (see + * {@code StaticTypesClosureWriter#addIntersectionMarkers}), which the one shared + * {@code PackedClosure} adapter cannot do per-literal — so keep it a class. The marker list is + * recorded by the type checker; a non-empty one is the signal. + */ + private static boolean isIntersectionTyped(final ClosureExpression expression) { + Object markers = expression.getNodeMetaData(org.codehaus.groovy.transform.stc.StaticTypesMarker.LAMBDA_MARKERS); + return markers instanceof List && !((List<?>) markers).isEmpty(); + } + + /** + * Whether the closure is being compiled inside a trait. A trait's method bodies (and their + * closures) are moved into a static {@code $Trait$Helper} nested class whose {@code this} is a + * synthetic {@code $self} parameter, not an instance receiver — a shape the packed dispatch + * codegen does not reproduce (it produced invalid bytecode). Decline and keep the closure as a + * class until the trait context is supported. Detected by walking the enclosing class's outer + * chain to the {@code @Trait}-annotated interface. + */ + private boolean isInTraitContext() { + for (ClassNode c = controller.getClassNode(); c != null; c = c.getOuterClass()) { + if (org.codehaus.groovy.transform.trait.Traits.isTrait(c)) return true; + } + return false; + } + + /** + * Whether a packable, non-escaping closure literal should actually be packed. Dynamic compilation + * has no proof of delegate-independence, so the only trigger is the {@code @PackedClosures} trust + * assertion; {@code StaticTypesClosureWriter} overrides this to add the + * {@code groovy.target.closure.pack} flag and the delegate-independence proof. + * + * @param annotated whether a non-DISABLED {@code @PackedClosures} is in scope + */ + protected boolean isPackTriggered(final ClosureExpression expression, final boolean annotated) { + if (annotated) return true; + // Dynamic compilation has no delegate-independence proof from the type checker, so the + // annotation is normally the only trigger. But a closure with NO free names -- no + // implicit-this call, no unqualified/dynamic variable -- cannot be affected by any + // caller-set delegate at all, so it is delegate-independent by syntax alone, no types + // needed. The flag auto-packs exactly that provably-safe subset (GROOVY-12151 dynamic + // syntactic path); a set delegate on such a closure is a harmless no-op, so it needs no + // strict runtime guard (marked here for writePackedClosure). + if (SystemUtil.getBooleanSafe("groovy.target.closure.pack") && isSyntacticallyDelegateIndependent(expression)) { + expression.putNodeMetaData(SYNTACTIC_PACK, Boolean.TRUE); + return true; + } + return false; + } + + /** Metadata marking a closure packed because it is syntactically delegate-independent (no strict guard). */ + private static final String SYNTACTIC_PACK = "org.codehaus.groovy.classgen.asm.ClosureWriter.syntacticPack"; + + /** + * Whether the closure's body contains no name a caller-set delegate could intercept — no + * implicit-this method call or property access, and no unqualified/dynamic variable (a free + * name the runtime would resolve through the owner/delegate chain). Only parameters, locals, + * captured variables, constants and explicit/parameter receivers remain, none of which a + * delegate can touch, so such a closure is delegate-independent by construction regardless of + * types. Nested closures are walked too (a free name anywhere carries the delegate chain). + * Conservative: any doubt returns {@code false} (keep the closure as a class). + */ + private boolean isSyntacticallyDelegateIndependent(final ClosureExpression expression) { + Statement code = expression.getCode(); + if (code == null) return false; + boolean[] dependent = {false}; + code.visit(new CodeVisitorSupport() { + @Override public void visitMethodCallExpression(final MethodCallExpression call) { + if (call.isImplicitThis()) dependent[0] = true; // foo() -> could be a delegate method + super.visitMethodCallExpression(call); + } + @Override public void visitPropertyExpression(final PropertyExpression pexp) { + // implicit-this (bar) could be a delegate property; explicit this.bar inside a + // dynamic closure routes through the MOP (getProperty -- where Map-owner entry + // semantics and metaclass interception live), which a hoisted body's direct + // field shortcut would bypass + Expression obj = pexp.getObjectExpression(); + if (pexp.isImplicitThis() + || (obj instanceof VariableExpression && ((VariableExpression) obj).isThisExpression())) { + dependent[0] = true; + } + super.visitPropertyExpression(pexp); + } + @Override public void visitVariableExpression(final VariableExpression ve) { + // only a parameter/local binding is safe: a bare name bound to an owner FIELD or + // PROPERTY by VariableScopeVisitor is still a free name at runtime -- a dynamic + // closure resolves it through the delegate chain first under DELEGATE_FIRST, and + // through the MOP (e.g. an ExpandoMetaClass property) -- as is a DynamicVariable + Variable av = ve.getAccessedVariable(); + if (av instanceof DynamicVariable || av instanceof FieldNode || av instanceof PropertyNode) { + dependent[0] = true; + } + super.visitVariableExpression(ve); + } + }); + return !dependent[0]; + } + + /** + * A trigger-specific decline reason for {@link #declineReason}, or {@code null} if the trigger did + * not decline this closure. Only the static writer has one (a delegate-resolved body); the dynamic + * path trusts the annotation and never declines on trigger grounds. + */ + protected String triggerDeclineReason(final ClosureExpression expression) { + return null; + } + + /** Whether the hoisted body is compiled statically (true only for {@code @CompileStatic}). */ + protected boolean compilesHoistedBodyStatically() { + return false; + } + + /** + * Whether the packed adapter installs the runtime delegate guard. The dynamic trust path needs it + * (an unverifiable assertion must fail fast on misuse); the static writer proved independence, so + * it overrides this to {@code false} and a caller-set delegate is then stored and ignored. + */ + protected boolean packedClosureUsesDelegateGuard() { + return true; + } + + /** The closure's inferred return type, normalised the same way {@code createClosureClass} does for doCall. */ + private static ClassNode inferredClosureReturnType(final ClosureExpression expression) { + ClassNode returnType = expression.getNodeMetaData(INFERRED_RETURN_TYPE); + if (returnType == null) returnType = ClassHelper.OBJECT_TYPE; + else if (returnType.isPrimaryClassNode()) returnType = returnType.getPlainNodeReference(); + else if (ClassHelper.isPrimitiveType(returnType)) returnType = ClassHelper.getWrapper(returnType); + else if (GenericsUtils.hasUnresolvedGenerics(returnType)) returnType = GenericsUtils.nonGeneric(returnType); + return returnType; + } + + /** + * The erasure of a captured variable's type, safe to declare on the hoisted method: generic + * placeholders are replaced by their bound (the hoisted method does not declare the enclosing + * method's type variables) and type arguments are dropped ({@code List<T>} → {@code List}). + */ + private static ClassNode erasedType(final ClassNode type) { + ClassNode t = type; + if (t == null) return ClassHelper.OBJECT_TYPE; + if (t.isGenericsPlaceHolder()) t = t.redirect(); + return t.getPlainNodeReference(); + } + + /** + * Reports a top-level closure that was NOT packed, per the {@code mode} of the enclosing + * {@code @PackedClosures} annotation (WARN => compiler warning, STRICT => compiler error). The + * automatic {@code groovy.target.closure.pack} path is lenient by default, but setting + * {@code groovy.target.closure.pack.report=true} alongside it surfaces every decline (with its + * reason) as a warning — the operational way to see where the packability boundary falls in a + * real codebase before opting scopes in or out. A nested closure is a structural decline (its + * enclosing context is a generated function, not the owner class), so it is not reported. + */ + private void reportUnpacked(final ClosureExpression expression) { + if (controller.isInGeneratedFunction()) return; + PackMode mode = annotatedMode(); + if (mode == null) { + if (SystemUtil.getBooleanSafe("groovy.target.closure.pack") + && SystemUtil.getBooleanSafe("groovy.target.closure.pack.report")) { + controller.getSourceUnit().addWarning(WarningMessage.LIKELY_ERRORS, + "closure packing: closure was not packed -- " + declineReason(expression), expression); + } + return; + } + // LENIENT/DISABLED are both silent -- DISABLED asked NOT to pack, so a decline is expected + if (mode == PackMode.LENIENT || mode == PackMode.DISABLED) return; + String msg = "@PackedClosures: closure was not packed -- " + declineReason(expression); + if (mode == PackMode.STRICT) { + controller.getSourceUnit().getErrorCollector() + .addErrorAndContinue(msg, expression, controller.getSourceUnit()); + } else { + // WARN is opt-in, so emit at LIKELY_ERRORS to show at the default warning level (the plain + // addWarning(text, node) uses POSSIBLE_ERRORS, which the default level suppresses). groovyc + // surfaces collected warnings on success since GROOVY-12132; Gradle/IDEs already do. + controller.getSourceUnit().addWarning(WarningMessage.LIKELY_ERRORS, msg, expression); + } + } + + /** The most specific {@code @PackedClosures} mode in scope (method wins over class), or null if not annotated. */ + private PackMode annotatedMode() { + MethodNode m = controller.getMethodNode(); + PackMode mode = (m != null) ? packModeOf(m.getAnnotations()) : null; + if (mode != null) return mode; + for (ClassNode c = controller.getClassNode(); c != null; c = c.getOuterClass()) { + mode = packModeOf(c.getAnnotations()); + if (mode != null) return mode; + } + return null; + } + + /** The mode of a {@code @PackedClosures} in the given set (LENIENT if present without an explicit mode), or null. */ + private static PackMode packModeOf(final List<AnnotationNode> annotations) { + for (AnnotationNode a : annotations) { + if (a.getClassNode() != null && "groovy.transform.PackedClosures".equals(a.getClassNode().getName())) { + Expression member = a.getMember("mode"); + if (member instanceof PropertyExpression) { + try { + return PackMode.valueOf(((PropertyExpression) member).getPropertyAsString()); + } catch (IllegalArgumentException ignore) { + // malformed member; treat as the default + } + } + return PackMode.LENIENT; + } + } + return null; + } + + /** Why {@link #chooseStrategy} declined this (packable, top-level) closure -- the message WARN/STRICT report. */ + private String declineReason(final ClosureExpression expression) { + if (!isPackable(expression)) { + return "it needs a real Closure (uses owner/delegate/thisObject/resolveStrategy/super or metaClass, " + + "has default parameter values, or contains an anonymous inner class)"; + } + String triggerReason = triggerDeclineReason(expression); + if (triggerReason != null) return triggerReason; + if (escapesEnclosingMethod(expression)) { + return "it escapes its method (returned, stored to a field/property/index, appended, or in a collection literal)"; + } + if (serializationBound(expression)) { + return "it is visibly serialization-bound (cast or coerced to a Serializable type, or passed directly " + + "to writeObject) and packed closures are not serializable"; + } + if (isFieldInitializer(expression)) { + return "it initialises a field (a visible escape: the closure outlives its construction context)"; + } + if (isIntersectionTyped(expression)) { + return "it is cast to an intersection type, whose marker interfaces the shared adapter cannot declare"; + } + if (isInTraitContext()) { + return "it is declared inside a trait, whose helper-class context packed dispatch does not yet support"; + } + if (controller.getCompileStack().isInSpecialConstructorCall()) { + return "it is an argument to a this(...)/super(...) constructor call, where the enclosing instance " + + "is not yet initialised, so it cannot be the packed adapter's owner"; + } + return "it is not eligible for packing in this scope"; + } + + /** + * Conservative compile-time escape gate. A packed closure is bound to the owner and backed by a + * shared adapter that fails fast if a delegate is later set on it. To avoid that surprise for the + * clearest cases, decline (keep a real closure class for) a closure literal that visibly escapes + * the method it is written in: stored into a field/property/index (e.g. {@code attrs.optionValue = + * { ... }}), returned, appended with {@code <<}, or placed in a list/map literal. Transitive escapes + * (via a local that is later returned) are intentionally not chased here; the runtime guard remains + * the backstop for those. + */ + private boolean escapesEnclosingMethod(final ClosureExpression expression) { + MethodNode m = controller.getMethodNode(); + if (m == null || m.getCode() == null) return false; + return escapingClosuresByMethod + .computeIfAbsent(m, k -> collectEscapingClosures(k.getCode())) + .contains(expression); + } + + private static Set<ClosureExpression> collectEscapingClosures(final Statement code) { + Set<ClosureExpression> escaping = Collections.newSetFromMap(new IdentityHashMap<>()); + code.visit(new CodeVisitorSupport() { + private void mark(final Expression e) { + if (e instanceof ClosureExpression) escaping.add((ClosureExpression) e); + } + @Override public void visitReturnStatement(final ReturnStatement statement) { + mark(statement.getExpression()); + super.visitReturnStatement(statement); + } + @Override public void visitBinaryExpression(final BinaryExpression be) { + int op = be.getOperation().getType(); + if (op == Types.EQUAL && !isLocalTarget(be.getLeftExpression())) { + mark(be.getRightExpression()); // assigned to a field/property/index + } else if (op == Types.LEFT_SHIFT) { + mark(be.getRightExpression()); // appended, e.g. list << { ... } + } + super.visitBinaryExpression(be); + } + @Override public void visitListExpression(final ListExpression le) { + le.getExpressions().forEach(this::mark); + super.visitListExpression(le); + } + @Override public void visitMapEntryExpression(final MapEntryExpression me) { + mark(me.getValueExpression()); + super.visitMapEntryExpression(me); + } + }); + return escaping; + } + + /** True when an assignment target is a plain local variable (not a field/property that escapes). */ + private static boolean isLocalTarget(final Expression lhs) { + if (!(lhs instanceof VariableExpression)) return false; // property/field/index target + Variable v = ((VariableExpression) lhs).getAccessedVariable(); + return !(v instanceof FieldNode) && !(v instanceof PropertyNode); + } + + /** + * Conservative compile-time serialization gate, the same shape as the escape gate: a packed + * closure is not serializable, so decline (keep a real closure class for) a closure literal + * that is <em>visibly</em> serialization-bound — cast or coerced ({@code as}) to a type that is + * or implements {@code Serializable} (including serializable SAM coercions, whose proxy carries + * the closure), or passed directly as an argument to a {@code writeObject} call. Transitive + * routes (a local later serialized, a callee that serializes its argument) are intentionally + * not chased; the adapter's fail-fast {@code writeObject} remains the backstop for those. + * Declines are reported by {@code @PackedClosures(mode = WARN | STRICT)} like any other. + */ + private boolean serializationBound(final ClosureExpression expression) { + MethodNode m = controller.getMethodNode(); + if (m == null || m.getCode() == null) return false; + return serializationBoundByMethod + .computeIfAbsent(m, k -> collectSerializationBoundClosures(k.getCode())) + .contains(expression); + } + + private static Set<ClosureExpression> collectSerializationBoundClosures(final Statement code) { + Set<ClosureExpression> bound = Collections.newSetFromMap(new IdentityHashMap<>()); + // pass 1: locals directly assigned a closure literal, so pass 2 can follow the canonical + // one-hop shape `def c = { ... }; out.writeObject(c)` (a local reassigned a second literal + // maps to both -- conservative: either literal declines) + Map<String, List<ClosureExpression>> literalsByLocal = new HashMap<>(); + code.visit(new CodeVisitorSupport() { + @Override public void visitBinaryExpression(final BinaryExpression be) { + if (Types.isAssignment(be.getOperation().getType()) + && be.getLeftExpression() instanceof VariableExpression + && isLocalTarget(be.getLeftExpression()) + && be.getRightExpression() instanceof ClosureExpression) { + literalsByLocal.computeIfAbsent(((VariableExpression) be.getLeftExpression()).getName(), + k -> new ArrayList<>()).add((ClosureExpression) be.getRightExpression()); + } + super.visitBinaryExpression(be); + } + }); + code.visit(new CodeVisitorSupport() { + private void mark(final Expression e) { + if (e instanceof ClosureExpression) { + bound.add((ClosureExpression) e); + } else if (e instanceof VariableExpression) { + List<ClosureExpression> held = literalsByLocal.get(((VariableExpression) e).getName()); + if (held != null) bound.addAll(held); + } + } + @Override public void visitCastExpression(final CastExpression ce) { + ClassNode target = ce.getType(); + if (ClassHelper.SERIALIZABLE_TYPE.equals(target) || target.implementsInterface(ClassHelper.SERIALIZABLE_TYPE)) { + mark(ce.getExpression()); + } + super.visitCastExpression(ce); + } + @Override public void visitMethodCallExpression(final MethodCallExpression mce) { + if ("writeObject".equals(mce.getMethodAsString()) && mce.getArguments() instanceof TupleExpression) { + ((TupleExpression) mce.getArguments()).getExpressions().forEach(this::mark); + } + super.visitMethodCallExpression(mce); + } + }); + return bound; + } + + /** + * Spike packability gate: pack read-only closures (including nested and captures). Captured-variable + * writes are supported via Reference threading for a flat closure only (declined if the body also + * contains a nested closure, or uses an unsupported compound-assignment operator). Anything needing a + * real Closure — delegate/owner/thisObject/resolveStrategy or {@code super} — is declined. + */ + private static boolean isPackable(final ClosureExpression expression) { + Statement code = expression.getCode(); + if (code == null) return false; + // Default parameter values generate arity-overloaded doCall methods on a closure class; + // the single hoisted method cannot reproduce that dispatch, so decline. + if (expression.isParameterSpecified()) { + for (Parameter p : expression.getParameters()) { + if (p.hasInitialExpression()) return false; + } + } + boolean[] ok = {true}; + code.visit(new CodeVisitorSupport() { + @Override public void visitClosureExpression(final ClosureExpression e) { + // A nested closure constructed inside the hoisted method gets the enclosing INSTANCE + // as its owner instead of the (packed) outer closure object. That is observationally + // equivalent for owner-chain resolution, but not for a nested body that names + // owner/delegate/thisObject explicitly -- so the outer packs only if every nested + // closure is itself packable. + if (!isPackable(e)) ok[0] = false; + } + @Override public void visitVariableExpression(final VariableExpression ve) { + if (ve.isSuperExpression() || FORBIDDEN_CLOSURE_NAMES.contains(ve.getName())) ok[0] = false; + } + @Override public void visitMethodCallExpression(final MethodCallExpression call) { + // implicit-this accessor/mutator forms of the same pseudo-properties, e.g. getDelegate() + if (call.isImplicitThis() && FORBIDDEN_CLOSURE_CALLS.contains(call.getMethodAsString())) ok[0] = false; + super.visitMethodCallExpression(call); + } + @Override public void visitConstructorCallExpression(final ConstructorCallExpression cce) { + // an anonymous inner class in the body is generated against the enclosing closure + // class (its outer instance); hoisting would hand it the wrong enclosing instance + if (cce.isUsingAnonymousInnerClass()) ok[0] = false; + super.visitConstructorCallExpression(cce); + } + }); + // All write forms to captured variables are supported: the shared Reference is passed as a + // holder parameter, so the ASM generator emits the implicit get()/set() exactly as it does + // for a closure class -- no operator restrictions. + return ok[0]; + } + + private static Set<String> capturedNames(final ClosureExpression expression) { + Set<String> captured = new HashSet<>(); + VariableScope vs = expression.getVariableScope(); + if (vs != null) { + for (Iterator<Variable> it = vs.getReferencedLocalVariablesIterator(); it.hasNext(); ) { + captured.add(it.next().getName()); + } + } + return captured; + } + + /** + * Captured variables that must be threaded as the shared {@code groovy.lang.Reference} (holder) + * rather than by value: those written (reassigned) anywhere in the enclosing method, and those a + * <em>nested</em> closure inside the body also captures. The nested case is a bytecode-shape + * requirement, not a mutation one: a nested closure that compiles as a class takes the shared + * Reference in its constructor (every captured local is Reference-boxed in the class world), so a + * by-value slot would fail verification there; a nested closure that itself packs simply + * dereferences the holder. The enclosing frame always has the Reference to pass — any local + * captured by a closure is boxed in its declaring frame. + */ + private Set<String> holderCaptureNames(final ClosureExpression expression) { + Set<String> captured = capturedNames(expression); + Set<String> holder = new HashSet<>(collectWrittenNames(expression.getCode())); + MethodNode m = controller.getMethodNode(); + if (m != null && m.getCode() != null) { + holder.addAll(writtenNamesByMethod.computeIfAbsent(m, k -> collectWrittenNames(k.getCode()))); + } + Statement code = expression.getCode(); + if (code != null) { + code.visit(new CodeVisitorSupport() { + @Override public void visitClosureExpression(final ClosureExpression nested) { + VariableScope nvs = nested.getVariableScope(); + if (nvs != null) { + for (Iterator<Variable> it = nvs.getReferencedLocalVariablesIterator(); it.hasNext(); ) { + holder.add(it.next().getName()); + } + } + super.visitClosureExpression(nested); + } + }); + } + holder.retainAll(captured); + return holder; + } + + /** Names assigned or incremented in the given code (declarations with initializers excluded). */ + private static Set<String> collectWrittenNames(final Statement code) { + Set<String> written = new HashSet<>(); + if (code == null) return written; + code.visit(new CodeVisitorSupport() { // descends into nested closures + @Override public void visitBinaryExpression(final BinaryExpression be) { + if (Types.isAssignment(be.getOperation().getType()) + && !(be instanceof org.codehaus.groovy.ast.expr.DeclarationExpression) + && be.getLeftExpression() instanceof VariableExpression) { + written.add(((VariableExpression) be.getLeftExpression()).getName()); + } + super.visitBinaryExpression(be); + } + @Override public void visitPostfixExpression(final PostfixExpression pe) { + recordIncrement(pe.getExpression()); + super.visitPostfixExpression(pe); + } + @Override public void visitPrefixExpression(final PrefixExpression pe) { + recordIncrement(pe.getExpression()); + super.visitPrefixExpression(pe); + } + private void recordIncrement(final Expression operand) { + if (operand instanceof VariableExpression) { + written.add(((VariableExpression) operand).getName()); + } + } + }); + return written; + } + + // NB: metaClass resolves against the closure object ITSELF (per-closure-class identity), which + // the shared adapter cannot reproduce, so it declines like the other closure pseudo-properties. + private static final Set<String> FORBIDDEN_CLOSURE_NAMES = + new HashSet<>(java.util.Arrays.asList("owner", "delegate", "thisObject", "directive", + "resolveStrategy", "metaClass")); + + /** Accessor/mutator forms of the same pseudo-properties, called with implicit this. */ + private static final Set<String> FORBIDDEN_CLOSURE_CALLS = + new HashSet<>(java.util.Arrays.asList("getOwner", "getDelegate", "getThisObject", "getDirective", + "getResolveStrategy", "setDelegate", "setDirective", "setResolveStrategy", + "getMaximumNumberOfParameters", "getParameterTypes", "getMetaClass", "setMetaClass", + "invokeMethod", "getProperty", "setProperty")); + + /** + * Rebinds captured-variable references in the moved body to the hoisted method's parameters, + * matched by the accessed {@link Variable}'s <em>identity</em> rather than by name. Groovy forbids + * shadowing an in-scope local/parameter, so a name match would be correct today; keying on identity + * keeps it correct even if that scoping rule ever relaxed, and never rewrites an unrelated binding + * that happens to share a captured name. + */ + private static void rebindCaptured(final Statement body, final Map<Variable, Parameter> capturedParams) { + body.visit(new CodeVisitorSupport() { + @Override public void visitVariableExpression(final VariableExpression ve) { + Parameter p = capturedParams.get(ve.getAccessedVariable()); + if (p != null) { + ve.setAccessedVariable(p); + // plain read-only captures become by-value params; holder-threaded captures + // (written, or re-captured by a nested closure) stay closure-shared so the + // ASM generator routes them through the Reference + ve.setClosureSharedVariable(p.isClosureSharedVariable()); + } + } + }); + } + + /** + * Emits a packed closure: hoists the body to a synthetic method on the enclosing class (captured + * variables become leading, by-value parameters) and constructs a shared {@code PackedClosure} + * adapter that dispatches to it — no inner class. + */ + private void writePackedClosure(final ClosureExpression expression) { + ClassNode enclosing = controller.getClassNode(); + MethodVisitor mv = controller.getMethodVisitor(); + AsmClassGenerator acg = controller.getAcg(); + OperandStack os = controller.getOperandStack(); + + List<String> captured = new ArrayList<>(); + Map<String, ClassNode> capturedTypes = new HashMap<>(); + Map<String, Variable> capturedVars = new HashMap<>(); // name -> the original captured variable + VariableScope vs = expression.getVariableScope(); + if (vs != null) { + for (Iterator<Variable> it = vs.getReferencedLocalVariablesIterator(); it.hasNext(); ) { + Variable v = it.next(); + captured.add(v.getName()); + capturedTypes.put(v.getName(), erasedType(v.getOriginType())); + capturedVars.put(v.getName(), v); + } + } + + // An EMPTY parameter array is the implicit-it literal ({ ... }); null is the explicit + // zero-parameter form ({ -> ... }), which must report arity 0 (GString's writer-vs-call + // branch and SAM matching key on it), exactly as its generated class would. + Parameter[] declared = expression.getParameters(); + boolean implicitParam = (declared != null && declared.length == 0); + Parameter[] closureParams = implicitParam + ? new Parameter[]{new Parameter(ClassHelper.OBJECT_TYPE, "it")} + : (declared != null ? declared : Parameter.EMPTY_ARRAY); + int arity = closureParams.length; + + Set<String> holderThreaded = holderCaptureNames(expression); + Parameter[] methodParams = new Parameter[captured.size() + closureParams.length]; + Map<Variable, Parameter> capturedByVar = new IdentityHashMap<>(); // original variable -> hoisted param + boolean typedBody = compilesHoistedBodyStatically(); + for (int i = 0; i < captured.size(); i++) { + String cn = captured.get(i); + boolean isWritten = holderThreaded.contains(cn); + Parameter p; + if (isWritten) { + // A written capture arrives as the SHARED groovy.lang.Reference itself: declare the + // parameter exactly like a closure-class constructor does (originType = value type, + // descriptor type = Reference, closure-shared + use-existing-reference), so + // CompileStack.init registers it as a holder and the ASM generator emits the implicit + // get()/set() on every read/write -- no AST rewriting, and the untouched body keeps + // the STC metadata that lets it compile statically. + ClassNode vt = capturedTypes.get(cn); + if (ClassHelper.isPrimitiveType(vt)) vt = ClassHelper.getWrapper(vt); + p = new Parameter(vt, cn); + p.setType(ClassHelper.makeReference()); + p.setClosureSharedVariable(true); + p.putNodeMetaData(UseExistingReference.class, Boolean.TRUE); + // the static writer's type chooser must see the VALUE type: the holder load already + // dereferences, so resolving this variable as Reference would add a bogus checkcast + p.putNodeMetaData(org.codehaus.groovy.transform.stc.StaticTypesMarker.INFERRED_TYPE, vt); + } else { + // For a statically-compiled body, type the read-only capture from its declared type so + // the loads match the STC metadata already on the body expressions (otherwise the + // static writer falls back to dynamic dispatch on the type mismatch). + // an untyped capture may carry a flow-inferred type STC recorded on the original + // variable (e.g. an enclosing closure's inferred parameter, captured by a nested + // literal whose method reference expects the narrowed type). DECLARE the hoisted + // parameter by it: the dispatch table's case checkcasts to declared parameter + // types, so the body's load then carries the type the use sites were compiled + // against — where a plain Object parameter loads unverifiably wide (the classed + // emission gets the equivalent cast on its shared-Reference dereference). + ClassNode capturedType = typedBody ? capturedTypes.get(cn) : ClassHelper.OBJECT_TYPE; + Variable v = capturedVars.get(cn); + if (typedBody && v instanceof org.codehaus.groovy.ast.ASTNode) { + Object inferred = ((org.codehaus.groovy.ast.ASTNode) v) + .getNodeMetaData(org.codehaus.groovy.transform.stc.StaticTypesMarker.INFERRED_TYPE); + if (inferred instanceof ClassNode) { + capturedType = erasedType((ClassNode) inferred); + } + } + p = new Parameter(capturedType, cn); + } + methodParams[i] = p; + capturedByVar.put(capturedVars.get(cn), p); + } + System.arraycopy(closureParams, 0, methodParams, captured.size(), closureParams.length); + + Statement body = expression.getCode(); + rebindCaptured(body, capturedByVar); // read-only -> plain param; written -> holder param + + // In a static method there is no `this`: the owner is the enclosing class and the hoisted + // method must be static (loaded/dispatched against the class, not local slot 0). + boolean staticContext = controller.isStaticMethod(); + List<MethodNode> targets = packedTargets(enclosing); + int id = targets.size(); // ids are dense per class: the dispatcher emits a tableswitch over them + String name = PACKED_METHOD_PREFIX + id; + // PRIVATE, not public: the body is reached only through the class's $packedDispatch$ table + // (a tableswitch case invoking it directly via INVOKESPECIAL/INVOKESTATIC), and a private + // method is invoked exactly (no virtual dispatch), so an inherited packed closure never + // dispatches to a same-named hoisted method a subclass happens to declare (GROOVY-12151 review). + int mods = ACC_PRIVATE | ACC_SYNTHETIC | (staticContext ? ACC_STATIC : 0); + // Same return type a generated closure class would give doCall, so implicit return-value + // coercion (e.g. GString -> String for a Closure<String>) happens identically. This uses + // whatever the type checker recorded (@CompileStatic AND @TypeChecked -- the dynamically + // compiled body coerces at return like any declared-return dynamic method), falling back + // to Object when no inference ran. + ClassNode hoistedReturnType = inferredClosureReturnType(expression); + MethodNode hoisted = new MethodNode(name, mods, hoistedReturnType, + methodParams, ClassNode.EMPTY_ARRAY, body); + // Added after the ReturnAdder phase, so run it ourselves: it wires implicit returns through + // every statement shape (trailing expressions, if/else branches, try/catch, ...). + new org.codehaus.groovy.classgen.ReturnAdder().visitMethod(hoisted); + // Typed static dispatch (GEP-27 phase 4): the original body expressions were already visited + // by StaticCompilationVisitor as part of the enclosing method, so they carry the metadata the + // static writer needs (DIRECT_METHOD_CALL_TARGET, inferred types) -- including types that were + // only inferred (@ClosureParams, implicit it), which arrive for free via checkcasts. Written + // captures ride the holder machinery, so their bodies compile statically too. + hoisted.putNodeMetaData(STATIC_COMPILE_NODE, typedBody ? Boolean.TRUE : Boolean.FALSE); + addGeneratedMethod(enclosing, hoisted); + targets.add(hoisted); // claim the id; writePackedDispatcher renders the table at end of class + // GROOVY-12150 visibility: a packed closure has no generated class, so mark it with the hoisted + // method it became. The AST browser then renders it via the same path it uses for the generated + // closure class of an unpacked closure; the trailing () distinguishes a method from a class. + expression.putNodeMetaData(GENERATED_CLOSURE_CLASS, enclosing.getName() + "." + name + "()"); + + // Instantiate the fixed-arity family member matching the literal's declared parameter count + // (FixedN, with a varargs doCall, serves arities above four; the base class is abstract): + // the arity is then visible to class-level introspection — notably MetaClassHelper's + // SAM-overload disambiguation, which reflects the argument class's declared doCall — + // exactly as on a generated closure class. An implicit-parameter literal gets FixedIt + // (doCall() + doCall(Object)), matching the fuzzy "0 or 1" arity a generated class + // declares for it. A vararg-shaped literal (trailing array parameter, e.g. { ...z -> }) + // also gets FixedN: its generated class would declare a VARARG doCall, whose selection + // flexibility (zero args collect to an empty array where a fixed one-param doCall would + // null-fill) only a varargs declaration reproduces. + boolean varargShape = !implicitParam && arity > 0 && closureParams[arity - 1].getType().isArray(); + String pc = "org/codehaus/groovy/runtime/PackedClosure"; + if (implicitParam) pc = pc + "$FixedIt"; + else if (arity <= 4 && !varargShape) pc = pc + "$Fixed" + arity; + else pc = pc + "$FixedN"; + mv.visitTypeInsn(NEW, pc); + mv.visitInsn(DUP); + if (staticContext) { + new ClassExpression(enclosing).visit(acg); // owner = the enclosing class (static dispatch) + } else { + mv.visitVarInsn(ALOAD, 0); + os.push(ClassHelper.OBJECT_TYPE); // owner = this + } + // The class's shared dispatch table plus this body's compile-time id: dispatch is a plain + // interface call into a tableswitch (JIT-friendly, unlike a per-instance MethodHandle, which + // cannot be constant-folded), and the id binds the exact method so an inherited packed closure + // cannot misdispatch to a subclass's same-named method. + mv.visitMethodInsn(INVOKESTATIC, BytecodeHelper.getClassInternalName(enclosing), DISPATCHERS_GETTER, DISPATCHERS_GETTER_DESC, false); + os.push(ClassHelper.make(org.codehaus.groovy.runtime.GeneratedDispatcher.Bundle.class)); + BytecodeHelper.pushConstant(mv, id); + os.push(ClassHelper.int_TYPE); + mv.visitLdcInsn(name); + os.push(ClassHelper.STRING_TYPE); // method name (diagnostics only) + if (captured.isEmpty()) { + mv.visitInsn(ACONST_NULL); + os.push(ClassHelper.OBJECT_TYPE.makeArray()); + } else { + // Build Object[] of captured values: written vars pass their shared Reference (so writes + // propagate), read-only vars pass their value. + BytecodeHelper.pushConstant(mv, captured.size()); + os.push(ClassHelper.int_TYPE); + mv.visitTypeInsn(ANEWARRAY, "java/lang/Object"); + os.replace(ClassHelper.OBJECT_TYPE.makeArray()); + for (int i = 0; i < captured.size(); i++) { + String cn = captured.get(i); + mv.visitInsn(DUP); + os.push(ClassHelper.OBJECT_TYPE.makeArray()); + BytecodeHelper.pushConstant(mv, i); + os.push(ClassHelper.int_TYPE); + if (holderThreaded.contains(cn)) { + loadReference(cn, controller); // shared Reference holder + } else { + new VariableExpression(cn).visit(acg); // value + os.box(); + } + mv.visitInsn(AASTORE); + os.remove(3); + } + } + // The closure's declared parameter types, so getParameterTypes()-keyed caller behaviour + // (DGM arity decisions, vararg collection) matches a generated closure class exactly. + // A condy constant (resolved once per literal site, shared by every adapter created + // there — as a closure class shares its cached parameter-type array), NOT a fresh array: + // building one per creation measurably taxes creation-heavy code. The types travel as a + // method descriptor because primitive parameter types have no Class constant-pool form. + StringBuilder typesDescriptor = new StringBuilder("("); + for (int i = 0; i < arity; i++) { + typesDescriptor.append(BytecodeHelper.getTypeDescription(erasedType(closureParams[i].getType()))); + } + typesDescriptor.append(")V"); + mv.visitLdcInsn(new org.objectweb.asm.ConstantDynamic("paramTypes", "[Ljava/lang/Class;", + new org.objectweb.asm.Handle(org.objectweb.asm.Opcodes.H_INVOKESTATIC, DISPATCHER_TYPE, "paramTypes", + "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;)[Ljava/lang/Class;", Review Comment: this is still open and uncommented ########## src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java: ########## @@ -158,6 +262,1038 @@ public void writeClosure(final ClosureExpression expression) { controller.getOperandStack().replace(ClassHelper.CLOSURE_TYPE, localVariableParams.length); } + /** + * GEP-27 capability analysis: choose the compilation strategy for a closure literal. + * <p> + * A closure is packed (S1, {@link PackStrategy#PACKED_ADAPTER}) when it is <em>triggered</em> — + * either the enclosing scope opts in via {@code @PackedClosures} (the dynamic trust path), or the + * capability analysis <em>proves</em> it delegate-independent under {@code @CompileStatic} (the + * automatic path, see {@link #isDelegateIndependent}) — and it is in a packable position: + * it needs no real {@code Closure} semantics we cannot reproduce ({@link #isPackable}), it is + * written directly in a method rather than nested in another closure (owner-retargeting for + * nested closures is later work), and it does not visibly escape its method. Otherwise it stays a + * full generated class (S0, {@link PackStrategy#FULL_CLASS}) exactly as today. + */ + private PackStrategy chooseStrategy(final ClosureExpression expression) { + // The complete packability decision, in order (GEP-27 "The packability decision procedure"; + // any decline keeps the closure a generated class, exactly as today): + // 1. needs real Closure semantics (owner/delegate/thisObject/resolveStrategy/metaClass/ + // super, default parameter values, anonymous inner class)? + if (!isPackable(expression)) return PackStrategy.FULL_CLASS; + // 2. scope opted out? The most-specific @PackedClosures mode wins (method over class), and + // DISABLED beats an enclosing opt-in AND the flag. + PackMode mode = annotatedMode(); + if (mode == PackMode.DISABLED) return PackStrategy.FULL_CLASS; + boolean annotated = (mode != null); // LENIENT/WARN/STRICT all opt in (DISABLED handled above) + // 3. triggered and sound? annotation (dynamic trust), flag + syntactic no-free-name proof + // (dynamic), or flag/annotation + the type checker's delegate-independence proof (CS, + // which also declines mixed-mode dynamic islands and Map-owner property semantics). + if (!isPackTriggered(expression, annotated)) return PackStrategy.FULL_CLASS; + // 4. structural position: directly in a method of the owner class (nested closures await + // owner-retargeting)? + if (controller.isInGeneratedFunction()) return PackStrategy.FULL_CLASS; + // 5. visibly escapes (field/property/index store, return, <<, collection literal)? + if (escapesEnclosingMethod(expression)) return PackStrategy.FULL_CLASS; + // 6. visibly serialization-bound (Serializable cast/coercion, writeObject directly or via + // a literal-holding local)? + if (serializationBound(expression)) return PackStrategy.FULL_CLASS; + // 7. a field initializer (a field store the escape walk cannot see)? + if (isFieldInitializer(expression)) return PackStrategy.FULL_CLASS; + // 8.-10. a context the adapter cannot inhabit: intersection cast (per-literal marker + // interfaces), trait body ($Trait$Helper's synthetic receiver), or a this(...)/super(...) + // argument (uninitializedThis cannot be the dispatch receiver)? + if (isIntersectionTyped(expression)) return PackStrategy.FULL_CLASS; + if (isInTraitContext()) return PackStrategy.FULL_CLASS; + if (controller.getCompileStack().isInSpecialConstructorCall()) return PackStrategy.FULL_CLASS; + return PackStrategy.PACKED_ADAPTER; + } + + /** + * A closure in a field initializer ({@code def action = { ... }} at class level) is stored + * into a field by definition — the same escape the escape gate declines for in-method stores — + * but field initializers compile outside the enclosing method's visible code, so the escape + * walk cannot see them. Detected directly against the class's fields (identity match). + */ + private boolean isFieldInitializer(final ClosureExpression expression) { + for (FieldNode fn : controller.getClassNode().getFields()) { + if (fn.getInitialValueExpression() == expression) return true; + } + return false; + } + + /** + * A closure literal cast to an intersection type (e.g. {@code (Runnable & Serializable) { }}) + * needs its <em>generated class</em> to declare the marker interfaces (see + * {@code StaticTypesClosureWriter#addIntersectionMarkers}), which the one shared + * {@code PackedClosure} adapter cannot do per-literal — so keep it a class. The marker list is + * recorded by the type checker; a non-empty one is the signal. + */ + private static boolean isIntersectionTyped(final ClosureExpression expression) { + Object markers = expression.getNodeMetaData(StaticTypesMarker.LAMBDA_MARKERS); + return markers instanceof List && !((List<?>) markers).isEmpty(); + } + + /** + * Whether the closure is being compiled inside a trait. A trait's method bodies (and their + * closures) are moved into a static {@code $Trait$Helper} nested class whose {@code this} is a + * synthetic {@code $self} parameter, not an instance receiver — a shape the packed dispatch + * codegen does not reproduce (it produced invalid bytecode). Decline and keep the closure as a + * class until the trait context is supported. Detected by walking the enclosing class's outer + * chain to the {@code @Trait}-annotated interface. + */ + private boolean isInTraitContext() { + for (ClassNode c = controller.getClassNode(); c != null; c = c.getOuterClass()) { + if (Traits.isTrait(c)) return true; + } + return false; + } + + /** + * Whether a packable, non-escaping closure literal should actually be packed. Dynamic compilation + * has no proof of delegate-independence, so the only trigger is the {@code @PackedClosures} trust + * assertion; {@code StaticTypesClosureWriter} overrides this to add the + * {@code groovy.target.closure.pack} flag and the delegate-independence proof. + * + * @param annotated whether a non-DISABLED {@code @PackedClosures} is in scope + */ + protected boolean isPackTriggered(final ClosureExpression expression, final boolean annotated) { + if (annotated) return true; + // Dynamic compilation has no delegate-independence proof from the type checker, so the + // annotation is normally the only trigger. But a closure with NO free names -- no + // implicit-this call, no unqualified/dynamic variable -- cannot be affected by any + // caller-set delegate at all, so it is delegate-independent by syntax alone, no types + // needed. The flag auto-packs exactly that provably-safe subset (GROOVY-12151 dynamic + // syntactic path); a set delegate on such a closure is a harmless no-op, so it needs no + // strict runtime guard (marked here for writePackedClosure). + if (SystemUtil.getBooleanSafe(CompilerConfiguration.CLOSURE_PACKING) && isSyntacticallyDelegateIndependent(expression)) { + expression.putNodeMetaData(SYNTACTIC_PACK, Boolean.TRUE); + return true; + } + return false; + } + + /** Metadata marking a closure packed because it is syntactically delegate-independent (no strict guard). */ + private static final String SYNTACTIC_PACK = "org.codehaus.groovy.classgen.asm.ClosureWriter.syntacticPack"; + + /** + * Whether the closure's body contains no name a caller-set delegate could intercept — no + * implicit-this method call or property access, and no unqualified/dynamic variable (a free + * name the runtime would resolve through the owner/delegate chain). Only parameters, locals, + * captured variables, constants and explicit/parameter receivers remain, none of which a + * delegate can touch, so such a closure is delegate-independent by construction regardless of + * types. Nested closures are walked too (a free name anywhere carries the delegate chain). + * Conservative: any doubt returns {@code false} (keep the closure as a class). + */ + private boolean isSyntacticallyDelegateIndependent(final ClosureExpression expression) { + Statement code = expression.getCode(); + if (code == null) return false; + boolean[] dependent = {false}; + code.visit(new CodeVisitorSupport() { + @Override public void visitMethodCallExpression(final MethodCallExpression call) { + if (call.isImplicitThis()) dependent[0] = true; // foo() -> could be a delegate method + super.visitMethodCallExpression(call); + } + @Override public void visitPropertyExpression(final PropertyExpression pexp) { + // implicit-this (bar) could be a delegate property; explicit this.bar inside a + // dynamic closure routes through the MOP (getProperty -- where Map-owner entry + // semantics and metaclass interception live), which a hoisted body's direct + // field shortcut would bypass + Expression obj = pexp.getObjectExpression(); + if (pexp.isImplicitThis() + || (obj instanceof VariableExpression && ((VariableExpression) obj).isThisExpression())) { + dependent[0] = true; + } + super.visitPropertyExpression(pexp); + } + @Override public void visitVariableExpression(final VariableExpression ve) { + // only a parameter/local binding is safe: a bare name bound to an owner FIELD or + // PROPERTY by VariableScopeVisitor is still a free name at runtime -- a dynamic + // closure resolves it through the delegate chain first under DELEGATE_FIRST, and + // through the MOP (e.g. an ExpandoMetaClass property) -- as is a DynamicVariable + Variable av = ve.getAccessedVariable(); + if (av instanceof DynamicVariable || av instanceof FieldNode || av instanceof PropertyNode) { + dependent[0] = true; + } + super.visitVariableExpression(ve); + } + }); + return !dependent[0]; + } + + /** + * A trigger-specific decline reason for {@link #declineReason}, or {@code null} if the trigger did + * not decline this closure. Only the static writer has one (a delegate-resolved body); the dynamic + * path trusts the annotation and never declines on trigger grounds. + */ + protected String triggerDeclineReason(final ClosureExpression expression) { + return null; + } + + /** + * The declared type of a read-only (by-value) capture parameter on the hoisted method. The + * dynamic writer types it as {@code Object}; the static writer overrides this to the capture's + * declared/flow-inferred type so the body compiles with static dispatch. This is the seam that + * keeps @CompileStatic-specific typing out of the general (dynamic) emitter. + */ + protected ClassNode readOnlyCaptureType(final String name, final ClassNode declaredType, final Variable variable) { + return ClassHelper.OBJECT_TYPE; + } + + /** Marks the hoisted body's compilation mode; the static writer overrides this to compile it statically. */ + protected void markHoistedBody(final MethodNode hoisted) { + hoisted.putNodeMetaData(STATIC_COMPILE_NODE, Boolean.FALSE); + } + + /** + * Whether the packed adapter installs the runtime delegate guard. The dynamic trust path needs it + * (an unverifiable assertion must fail fast on misuse); the static writer proved independence, so + * it overrides this to {@code false} and a caller-set delegate is then stored and ignored. + */ + protected boolean packedClosureUsesDelegateGuard() { + return true; + } + + /** The closure's inferred return type, normalised the same way {@code createClosureClass} does for doCall. */ + private static ClassNode inferredClosureReturnType(final ClosureExpression expression) { + ClassNode returnType = expression.getNodeMetaData(INFERRED_RETURN_TYPE); + if (returnType == null) returnType = ClassHelper.OBJECT_TYPE; + else if (returnType.isPrimaryClassNode()) returnType = returnType.getPlainNodeReference(); + else if (ClassHelper.isPrimitiveType(returnType)) returnType = ClassHelper.getWrapper(returnType); + else if (GenericsUtils.hasUnresolvedGenerics(returnType)) returnType = GenericsUtils.nonGeneric(returnType); + return returnType; + } + + /** + * The erasure of a captured variable's type, safe to declare on the hoisted method: generic + * placeholders are replaced by their bound (the hoisted method does not declare the enclosing + * method's type variables) and type arguments are dropped ({@code List<T>} → {@code List}). + */ + protected static ClassNode erasedType(final ClassNode type) { + ClassNode t = type; + if (t == null) return ClassHelper.OBJECT_TYPE; + if (t.isGenericsPlaceHolder()) t = t.redirect(); + return t.getPlainNodeReference(); + } Review Comment: Would this not be better in one of the generics helper classes? Not a must. -- 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]
