paulk-asert commented on code in PR #2709:
URL: https://github.com/apache/groovy/pull/2709#discussion_r3648860426
##########
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:
Agreed — moved. It's now `GenericsUtils.erasure(ClassNode)`, sitting next to
its closest sibling `nonGeneric` (which strips type arguments but doesn't
resolve a placeholder to its bound — the part the hoisted signatures need, so
it wasn't quite reusable as-is). Both writers now call the shared helper.
--
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]
