Update ASM EventExecutor generator patch to respect event handler return types. Fixes #7311 (#7317)

Co-authored-by: Jan Boerman <janboerman95@gmail.com>
This commit is contained in:
Jan 2022-01-20 20:54:57 +01:00 committed by GitHub
parent 2a306f513c
commit feb72b8ae1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 13 additions and 9 deletions

View File

@ -6,7 +6,7 @@ Subject: [PATCH] Use ASM for event executors.
Uses method handles for private or static methods. Uses method handles for private or static methods.
diff --git a/build.gradle.kts b/build.gradle.kts diff --git a/build.gradle.kts b/build.gradle.kts
index 0e8e827b5a05ac9423386eaea4188d132cc7f954..70d6503745942ea8637146369aeab8f9357c42d5 100644 index 80fdd05dd593455ca89b66636ed30f1d9facf4ed..d8d459561cc75935136f8f9888ff27b45ad98f9e 100644
--- a/build.gradle.kts --- a/build.gradle.kts
+++ b/build.gradle.kts +++ b/build.gradle.kts
@@ -38,6 +38,9 @@ dependencies { @@ -38,6 +38,9 @@ dependencies {
@ -118,10 +118,10 @@ index 0000000000000000000000000000000000000000..c83672427324bd068ed52916f700b684
+} +}
diff --git a/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java diff --git a/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java
new file mode 100644 new file mode 100644
index 0000000000000000000000000000000000000000..b6e7d8ee8d903ebf975d60bec0e08603d9a49fdb index 0000000000000000000000000000000000000000..b8d5c13980858dc27fb5383726b7ebcaf14adcb8
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java +++ b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java
@@ -0,0 +1,47 @@ @@ -0,0 +1,51 @@
+package com.destroystokyo.paper.event.executor.asm; +package com.destroystokyo.paper.event.executor.asm;
+ +
+import java.lang.reflect.Method; +import java.lang.reflect.Method;
@ -147,14 +147,18 @@ index 0000000000000000000000000000000000000000..b6e7d8ee8d903ebf975d60bec0e08603
+ methodGenerator.returnValue(); + methodGenerator.returnValue();
+ methodGenerator.endMethod(); + methodGenerator.endMethod();
+ // Generate the execute method + // Generate the execute method
+ methodGenerator = new GeneratorAdapter(writer.visitMethod(ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Event;)V", null, null), ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Listener;)V");; + methodGenerator = new GeneratorAdapter(writer.visitMethod(ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Event;)V", null, null), ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Listener;)V");
+ methodGenerator.loadArg(0); + methodGenerator.loadArg(0);
+ methodGenerator.checkCast(Type.getType(m.getDeclaringClass())); + methodGenerator.checkCast(Type.getType(m.getDeclaringClass()));
+ methodGenerator.loadArg(1); + methodGenerator.loadArg(1);
+ methodGenerator.checkCast(Type.getType(m.getParameterTypes()[0])); + methodGenerator.checkCast(Type.getType(m.getParameterTypes()[0]));
+ methodGenerator.visitMethodInsn(m.getDeclaringClass().isInterface() ? INVOKEINTERFACE : INVOKEVIRTUAL, Type.getInternalName(m.getDeclaringClass()), m.getName(), Type.getMethodDescriptor(m), m.getDeclaringClass().isInterface()); + methodGenerator.visitMethodInsn(m.getDeclaringClass().isInterface() ? INVOKEINTERFACE : INVOKEVIRTUAL, Type.getInternalName(m.getDeclaringClass()), m.getName(), Type.getMethodDescriptor(m), m.getDeclaringClass().isInterface());
+ if (m.getReturnType() != void.class) { + // The only purpose of this switch statement is to generate the correct pop instruction, should the event handler method return something other than void.
+ methodGenerator.pop(); + // Non-void event handlers will be unsupported in a future release.
+ switch (Type.getType(m.getReturnType()).getSize()) {
+ // case 0 is omitted because the only type that has size 0 is void - no pop instruction needed.
+ case 1 -> methodGenerator.pop(); // handles reference types and most primitives
+ case 2 -> methodGenerator.pop2(); // handles long and double
+ } + }
+ methodGenerator.returnValue(); + methodGenerator.returnValue();
+ methodGenerator.endMethod(); + methodGenerator.endMethod();

View File

@ -5,7 +5,7 @@ Subject: [PATCH] Warn on strange @EventHandler return types
diff --git a/src/main/java/org/bukkit/plugin/EventExecutor.java b/src/main/java/org/bukkit/plugin/EventExecutor.java diff --git a/src/main/java/org/bukkit/plugin/EventExecutor.java b/src/main/java/org/bukkit/plugin/EventExecutor.java
index 9026e108ccd3a88aee1267ee275137befa646455..e5f10ed4312529c082acf7de44af3639f77c8b8e 100644 index 9026e108ccd3a88aee1267ee275137befa646455..e1860322ae0f3c35097d16767628744034941749 100644
--- a/src/main/java/org/bukkit/plugin/EventExecutor.java --- a/src/main/java/org/bukkit/plugin/EventExecutor.java
+++ b/src/main/java/org/bukkit/plugin/EventExecutor.java +++ b/src/main/java/org/bukkit/plugin/EventExecutor.java
@@ -51,6 +51,12 @@ public interface EventExecutor { @@ -51,6 +51,12 @@ public interface EventExecutor {
@ -15,8 +15,8 @@ index 9026e108ccd3a88aee1267ee275137befa646455..e5f10ed4312529c082acf7de44af3639
+ if (m.getReturnType() != Void.TYPE) { + if (m.getReturnType() != Void.TYPE) {
+ final org.bukkit.plugin.java.JavaPlugin plugin = org.bukkit.plugin.java.JavaPlugin.getProvidingPlugin(m.getDeclaringClass()); + final org.bukkit.plugin.java.JavaPlugin plugin = org.bukkit.plugin.java.JavaPlugin.getProvidingPlugin(m.getDeclaringClass());
+ org.bukkit.Bukkit.getLogger().warning("@EventHandler method " + m.getDeclaringClass().getName() + (Modifier.isStatic(m.getModifiers()) ? '.' : '#') + m.getName() + org.bukkit.Bukkit.getLogger().warning("@EventHandler method " + m.getDeclaringClass().getName() + (Modifier.isStatic(m.getModifiers()) ? '.' : '#') + m.getName()
+ + " returns non-void type " + m.getReturnType().getName() + ". This is an unsupported behavior which will be removed in a future version of Paper." + + " returns non-void type " + m.getReturnType().getName() + ". This is unsupported behavior and will no longer work in a future version of Paper."
+ + "This should be reported to the developers of " + plugin.getDescription().getFullName() + ", (" + String.join(",", plugin.getDescription().getAuthors()) + ')'); + + " This should be reported to the developers of " + plugin.getDescription().getFullName() + " (" + String.join(",", plugin.getDescription().getAuthors()) + ')');
+ } + }
if (Modifier.isStatic(m.getModifiers())) { if (Modifier.isStatic(m.getModifiers())) {
return new StaticMethodHandleEventExecutor(eventClass, m); return new StaticMethodHandleEventExecutor(eventClass, m);