[
https://issues.apache.org/jira/browse/GROOVY-11968?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18077007#comment-18077007
]
ASF GitHub Bot commented on GROOVY-11968:
-----------------------------------------
blackdrag commented on code in PR #2495:
URL: https://github.com/apache/groovy/pull/2495#discussion_r3159896730
##########
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java:
##########
@@ -87,15 +94,77 @@ public void init(final AsmClassGenerator asmClassGenerator,
final GeneratorConte
@Override
public void setMethodNode(final MethodNode mn) {
isInStaticallyCheckedMethod = isStaticallyCompiled(mn);
+ methodHasDynamicResolution = isInStaticallyCheckedMethod &&
hasDynamicResolution(mn);
super.setMethodNode(mn);
}
@Override
public void setConstructorNode(final ConstructorNode cn) {
isInStaticallyCheckedMethod = isStaticallyCompiled(cn);
+ methodHasDynamicResolution = isInStaticallyCheckedMethod &&
hasDynamicResolution(cn);
super.setConstructorNode(cn);
}
+ /**
+ * GROOVY-11968: returns {@code true} when the current statically compiled
method
+ * contains one or more sub-expressions that will be routed through the
regular
+ * (non-static) call site writer via {@link #getCallSiteWriterFor}. The
regular
+ * writer's per-method state must then be initialized at method entry.
+ */
+ public boolean methodHasDynamicResolution() {
+ return methodHasDynamicResolution;
+ }
+
+ private static boolean hasDynamicResolution(final MethodNode mn) {
+ if (mn == null) return false;
+ if (mn.getNodeMetaData(DYNAMIC_RESOLUTION) != null) return true;
+ if (mn.getCode() == null) return false;
+ var scanner = new DynamicResolutionScanner();
+ mn.getCode().visit(scanner);
+ return scanner.found;
+ }
+
+ private static class DynamicResolutionScanner extends CodeVisitorSupport {
+ boolean found;
+
+ @Override
+ public void visitMethodCallExpression(final MethodCallExpression call)
{
+ if (mark(call)) return;
+ super.visitMethodCallExpression(call);
+ }
+
+ @Override
+ public void visitStaticMethodCallExpression(final
StaticMethodCallExpression call) {
+ if (mark(call)) return;
+ super.visitStaticMethodCallExpression(call);
+ }
+
+ @Override
+ public void visitPropertyExpression(final PropertyExpression
expression) {
+ if (mark(expression)) return;
+ super.visitPropertyExpression(expression);
+ }
+
+ @Override
+ public void visitAttributeExpression(final AttributeExpression
expression) {
+ if (mark(expression)) return;
+ super.visitAttributeExpression(expression);
+ }
+
+ @Override
+ public void visitVariableExpression(final VariableExpression
expression) {
+ if (mark(expression)) return;
+ super.visitVariableExpression(expression);
+ }
+
+ private boolean mark(final Expression e) {
Review Comment:
not wanting to nitpick, but mark is something you do. This is a check.
isMakedDynamic or something like that would express the intend of the method
better
> SC: trait static field access generates invalid bytecode under indy=false
> (GROOVY-11907 follow-up)
> --------------------------------------------------------------------------------------------------
>
> Key: GROOVY-11968
> URL: https://issues.apache.org/jira/browse/GROOVY-11968
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> GROOVY-11817 introduced per-expression delegation between
> StaticTypesCallSiteWriter
> and the regular CallSiteWriter: a sub-expression marked DYNAMIC_RESOLUTION
> inside a
> @CompileStatic method is routed through the regular writer while the rest of
> the
> method stays on the static path. However,
> StaticTypesCallSiteWriter.makeSiteEntry()
> is a no-op, so the regular writer's per-method state (callSiteArrayVarIndex,
> the
> $getCallSiteArray() prologue, and the cached CallSite[] local slot) is never
> initialized. With invokedynamic this is harmless, since indy doesn't use the
> cached array. With indy=false, the regular writer's prepareCallSite() emits
> ALOAD against an unallocated slot, producing methods whose first instruction
> references a local beyond max_locals.
> This surfaces as VerifyError at class load. The most accessible reproducer is
> a
> @CompileStatic trait with a static field, compiled with
> groovy.target.indy=false:
> {code:groovy}
> @CompileStatic
> trait T {
> static double d = 1.0d
> static double bump() { d = d + 1.0d; d }
> }
> {code}
> T$Trait$Helper.bump and the static-field accessors are emitted with
> max_locals=1
> and a body that begins with dload_3, failing the verifier with
> "get long/double overflows locals". The same shape is broken for any
> wide-typed
> static trait field (long/double), and more generally for any
> DYNAMIC_RESOLUTION
> sub-expression in a statically compiled method when indy is disabled. Groovy 4
> is unaffected because pre-GROOVY-11817 the whole method ran on the regular
> writer and makeSiteEntry() always executed.
> The restructure for the original GROOVY-11907 (commit 19f38997a4) addresses
> the receiver-expression typing for the indy=true / global-AST-transform case,
> but does not address this handoff and is complementary to it.
> Proposed fix: ensure the regular CallSiteWriter is initialized for the current
> method whenever the method body contains any DYNAMIC_RESOLUTION-marked
> expressions. Concretely, in StaticTypesWriterController.setMethodNode (or
> StaticTypesCallSiteWriter.makeSiteEntry), detect the presence of
> DYNAMIC_RESOLUTION markers and call super.makeSiteEntry() so that the
> $getCallSiteArray() prologue is emitted and the cached-array local slot is
> allocated up front. No change to the trait transform, no change to opcode
> emission elsewhere; statically compiled methods with no DYNAMIC_RESOLUTION
> sub-expressions are unaffected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
