Commit 537a190c authored by Eric Bruneton's avatar Eric Bruneton
Browse files

Use a thread local to avoid having to override or call deprecated methods in...

Use a thread local to avoid having to override or call deprecated methods in MethodVisitor subclasses. See the comments in MethodVisitor. It is possible to get the same behavior by using an unused bit of 'opcode' as the 'preventRedirect' flag, but this would be a non-generic and hacky method.
parent f4858361
Pipeline #4305 passed with stage
in 11 minutes and 59 seconds
......@@ -436,23 +436,6 @@ public abstract class AdviceAdapter extends GeneratorAdapter implements Opcodes
}
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
mv.visitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
doVisitMethodInsn(opcode, descriptor);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -460,11 +443,14 @@ public abstract class AdviceAdapter extends GeneratorAdapter implements Opcodes
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
if (api < Opcodes.ASM5) {
preventRedirect();
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
doVisitMethodInsn(opcode, descriptor);
}
......
......@@ -279,22 +279,6 @@ public class AnalyzerAdapter extends MethodVisitor {
execute(opcode, 0, descriptor);
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -302,22 +286,14 @@ public class AnalyzerAdapter extends MethodVisitor {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
private void doVisitMethodInsn(
final int opcode,
final String owner,
final String name,
final String descriptor,
final boolean isInterface) {
if (mv != null) {
mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
if (api < Opcodes.ASM5) {
preventRedirect();
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
if (this.locals == null) {
labels = null;
return;
......
......@@ -111,22 +111,6 @@ public class CodeSizeEvaluator extends MethodVisitor implements Opcodes {
super.visitFieldInsn(opcode, owner, name, descriptor);
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -134,19 +118,10 @@ public class CodeSizeEvaluator extends MethodVisitor implements Opcodes {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
private void doVisitMethodInsn(
final int opcode,
final String owner,
final String name,
final String descriptor,
final boolean isInterface) {
if (opcode == INVOKEINTERFACE) {
minSize += 5;
maxSize += 5;
......@@ -154,9 +129,10 @@ public class CodeSizeEvaluator extends MethodVisitor implements Opcodes {
minSize += 3;
maxSize += 3;
}
if (mv != null) {
mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
if (api < Opcodes.ASM5) {
preventRedirect();
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
@Override
......
......@@ -506,22 +506,6 @@ public class InstructionAdapter extends MethodVisitor {
}
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -529,19 +513,10 @@ public class InstructionAdapter extends MethodVisitor {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
private void doVisitMethodInsn(
final int opcode,
final String owner,
final String name,
final String descriptor,
final boolean isInterface) {
switch (opcode) {
case Opcodes.INVOKESPECIAL:
invokespecial(owner, name, descriptor, isInterface);
......
......@@ -150,22 +150,6 @@ public class MethodRemapper extends MethodVisitor {
remapper.mapDesc(descriptor));
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -173,30 +157,19 @@ public class MethodRemapper extends MethodVisitor {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
private void doVisitMethodInsn(
final int opcode,
final String owner,
final String name,
final String descriptor,
final boolean isInterface) {
// Calling super.visitMethodInsn requires to call the correct version depending on this.api
// (otherwise infinite loops can occur). To simplify and to make it easier to automatically
// remove the backward compatibility code, we inline the code of the overridden method here.
if (mv != null) {
mv.visitMethodInsn(
opcode,
remapper.mapType(owner),
remapper.mapMethodName(owner, name, descriptor),
remapper.mapMethodDesc(descriptor),
isInterface);
if (api < Opcodes.ASM5) {
preventRedirect();
}
super.visitMethodInsn(
opcode,
remapper.mapType(owner),
remapper.mapMethodName(owner, name, descriptor),
remapper.mapMethodDesc(descriptor),
isInterface);
}
@Override
......
......@@ -176,6 +176,7 @@ public class ClassFile {
* @param classContent the content of the class to load.
* @return a new instance of the class, or {@literal null} if the class is abstract, is an enum,
* or a module info.
* @throws Exception if the class is invalid or if an error occurs in its constructor.
*/
static Object newInstance(final String className, final byte[] classContent) throws Exception {
if (className.endsWith(MODULE_INFO)) {
......
......@@ -381,22 +381,6 @@ public class MethodNode extends MethodVisitor {
instructions.add(new FieldInsnNode(opcode, owner, name, descriptor));
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
instructions.add(new MethodInsnNode(opcode, owner, name, descriptor));
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -404,7 +388,7 @@ public class MethodNode extends MethodVisitor {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
......
......@@ -704,22 +704,6 @@ public class CheckMethodAdapter extends MethodVisitor {
++insnCount;
}
/**
* Deprecated.
*
* @deprecated use {@link #visitMethodInsn(int, String, String, String, boolean)} instead.
*/
@Deprecated
@Override
public void visitMethodInsn(
final int opcode, final String owner, final String name, final String descriptor) {
if (api >= Opcodes.ASM5) {
super.visitMethodInsn(opcode, owner, name, descriptor);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, opcode == Opcodes.INVOKEINTERFACE);
}
@Override
public void visitMethodInsn(
final int opcode,
......@@ -727,19 +711,10 @@ public class CheckMethodAdapter extends MethodVisitor {
final String name,
final String descriptor,
final boolean isInterface) {
if (api < Opcodes.ASM5) {
if (api < Opcodes.ASM5 && shouldRedirect()) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
return;
}
doVisitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
private void doVisitMethodInsn(
final int opcode,
final String owner,
final String name,
final String descriptor,
final boolean isInterface) {
checkVisitCodeCalled();
checkVisitMaxsNotCalled();
checkOpcodeMethod(opcode, Method.VISIT_METHOD_INSN);
......@@ -758,13 +733,10 @@ public class CheckMethodAdapter extends MethodVisitor {
throw new IllegalArgumentException(
"INVOKESPECIAL can't be used with interfaces prior to Java 8");
}
// Calling super.visitMethodInsn requires to call the correct version depending on this.api
// (otherwise infinite loops can occur). To simplify and to make it easier to automatically
// remove the backward compatibility code, we inline the code of the overridden method here.
if (mv != null) {
mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
if (api < Opcodes.ASM5) {
preventRedirect();
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
++insnCount;
}
......
......@@ -50,6 +50,201 @@ public abstract class MethodVisitor {
private static final String REQUIRES_ASM5 = "This feature requires ASM5";
/**
* A thread local flag used to redirect calls to deprecated methods. For instance, if a
* visitOldStuff method in API1 is deprecated and replaced with visitNewStuff in API2, then the
* redirection should be done as follows:
*
* <pre>
* public class StuffVisitor {
* ...
*
* &#64;Deprecated public void visitOldStuff(...) {
* if (api &#60; API2) preventRedirect();
* visitNewStuf(...);
* }
*
* public void visitNewStuff(...) {
* if (api &#60; API2 &#38;&#38; shouldRedirect()) {
* visitOldStuff(...);
* } else {
* [ do stuff ]
* }
* }
* }
* </pre>
*
* <p>If 'api' is equal to API2, there are two cases:
*
* <ul>
* <li>call visitNewStuff: the redirection test is skipped and 'do stuff' is executed directly.
* <li>call visitOldSuff: redirection is not prevented before calling visitNewStuff, but the
* redirection test is skipped anyway in visitNewStuff, which directly executes 'do stuff'.
* </ul>
*
* <p>If 'api' is equal to API1, there are two cases:
*
* <ul>
* <li>call visitOldSuff: redirection is prevented before calling visitNewStuff. Because of this
* visitNewStuff does not redirect back to visitOldStuff, and instead executes 'do stuff'.
* <li>call visitNewStuff: the call is redirected to visitOldStuff because redirection was not
* prevented. visitOldStuff now prevents redirection and calls visitNewStuff back. This time
* visitNewStuff does not redirect the call, and instead executes 'do stuff'.
* </ul>
*
* <h1>User subclasses</h1>
*
* <p>If a user subclass overrides one of these methods, there are only two cases: either 'api' is
* API1 and visitOldStuff is overridden (and visitNewStuff is not), or 'api' is API2 or more, and
* visitNewStuff is overridden (and visitOldStuff is not). Any other case is a user programming
* error.
*
* <p>If 'api' is equal to API2, the class hierarchy is equivalent to
*
* <pre>
* public class StuffVisitor {
* &#64;Deprecated public void visitOldStuff(...) { visitNewStuf(...); }
* public void visitNewStuff(...) { [ do stuff ] }
* }
* class UserStuffVisitor extends StuffVisitor {
* &#64;Override public void visitNewStuff(...) {
* super.visitNewStuff(...); // optional
* [ do user stuff ]
* }
* }
* </pre>
*
* <p>It is then obvious that whether visitNewStuff or visitOldStuff is called, 'do stuff' and 'do
* user stuff' will be executed, in this order.
*
* <p>If 'api' is equal to API1, the class hierarchy is equivalent to
*
* <pre>
* public class StuffVisitor {
* &#64;Deprecated public void visitOldStuff(...) {
* preventRedirect();
* visitNewStuf(...);
* }
* public void visitNewStuff(...) {
* if (shouldRedirect()) visitOldStuff(...); else [ do stuff ];
* }
* }
* class UserStuffVisitor extends StuffVisitor {
* &#64;Override public void visitOldStuff(...) {
* super.visitOldStuff(...); // optional
* [ do user stuff ]
* }
* }
* </pre>
*
* <p>and there are two cases:
*
* <ul>
* <li>call visitOldSuff: in the call to super.visitOldStuff, redirection is prevented and
* visitNewStuff is called. Here 'do stuff' is run because redirection was previously
* prevented, and execution eventually returns to UserStuffVisitor.visitOldStuff, where 'do
* user stuff' is run.
* <li>call visitNewStuff: the call is redirected to UserStuffVisitor.visitOldStuff because
* redirection was not prevented. Execution continues as in the previous case, resulting in
* 'do stuff' and 'do user stuff' being executed, in this order.
* </ul>
*
* <h1>ASM subclasses</h1>
*
* <p>In ASM packages, subclasses of StuffVisitor can typically be sub classed again by the user,
* and can be used with API1 or API2. Because of this, if such a subclass must override
* visitNewStuff, it must do so in the following way (and must not override visitOldStuff):
*
* <pre>
* public class AsmStuffVisitor extends StuffVisitor {
* &#64;Override public void visitNewStuff(...) {
* if (api &#60; API2 &#38;&#38; shouldRedirect()) {
* super.visitNewStuff(...);
* } else {
* if (api &#60; API2) preventRedirect(); // only needed if next line is needed
* super.visitNewStuff(...); // optional
* [ do other stuff ]
* }
* }
* }
* </pre>
*
* <p>If a user class extends this with 'api' equal to API2, the class hierarchy is equivalent to
*
* <pre>
* public class StuffVisitor {
* &#64;Deprecated public void visitOldStuff(...) { visitNewStuf(...); }
* public void visitNewStuff(...) { [ do stuff ] }
* }
* public class AsmStuffVisitor extends StuffVisitor {
* &#64;Override public void visitNewStuff(...) {
* super.visitNewStuff(...);
* [ do other stuff ]
* }
* }
* class UserStuffVisitor extends StuffVisitor {
* &#64;Override public void visitNewStuff(...) {
* super.visitNewStuff(...);
* [ do user stuff ]
* }
* }
* </pre>
*
* <p>It is then obvious that whether visitNewStuff or visitOldStuff is called, 'do stuff', 'do
* other stuff' and 'do user stuff' will be executed, in this order. If, on the other hand, a user
* class extends AsmStuffVisitor with 'api' equal to API1, the class hierarchy is equivalent to
*
* <pre>
* public class StuffVisitor {
* &#64;Deprecated public void visitOldStuff(...) {
* preventRedirect();
* visitNewStuf(...);
* }
* public void visitNewStuff(...) {
* if (shouldRedirect()) visitOldStuff(...); else [ do stuff ];
* }
* }
* public class AsmStuffVisitor extends StuffVisitor {
* &#64;Override public void visitNewStuff(...) {
* if (shouldRedirect()) {
* super.visitNewStuff(...);
* } else {
* preventRedirect();
* super.visitNewStuff(...);
* [ do other stuff ]
* }
* }
* }
* class UserStuffVisitor extends StuffVisitor {
* &#64;Override public void visitOldStuff(...) {
* super.visitOldStuff(...);
* [ do user stuff ]
* }
* }
* </pre>
*
* <p>and, here again, whether visitNewStuff or visitOldStuff is called, 'do stuff', 'do other
* stuff' and 'do user stuff' will be executed, in this order (exercise left to the reader).
*
* <h1>Notes</h1>
*
* <ul>
* <li>the PREVENT_REDIRECT flag is set only if 'api' is API1, just before calling
* visitNewStuff. By hypothesis, this method is not overridden by the user. And, in ASM,
* this method always calls shouldRedirect() before doing anything else, which resets the
* value of this flag. No exception can occur in between, and thus no 'finally' block is
* needed in visitOldStuff to reset the flag in exceptional cases. Another benefit is that
* 'do stuff' and 'do user stuff' can call visitOldStuff or visitNewStuff on a delegate
* visitor without any risk of side effects on the flag value (which might break the
* redirection logic). The last benefit is that this single flag can be used by all the
* deprecated methods for their redirection logic (no need of one flag per method).
* <li>the thread local flag is only accessed if 'api' is API1. In other words the associated
* runtime cost is only paid by users which have not updated their code to the latest API.
* <li>all the scenarios discussed above are unit tested in MethodVisitorTest.
* </ul>
*/
private static final ThreadLocal<Boolean> PREVENT_REDIRECT = new ThreadLocal<>();
/**
* The ASM API version implemented by this visitor. The value of this field must be one of {@link
* Opcodes#ASM4}, {@link Opcodes#ASM5}, {@link Opcodes#ASM6} or {@link Opcodes#ASM7}.
......@@ -85,6 +280,30 @@ public abstract class MethodVisitor {
this.mv = methodVisitor;
}
/**
* Prevents the next method call in the current thread from being redirected to its equivalent
* deprecated method. <i>FOR INTERNAL USE ONLY. This method can be removed or renamed in future
* ASM versions</i>.
*/
protected static void preventRedirect() {
PREVENT_REDIRECT.set(Boolean.TRUE);
}
/**
* Returns true if the current method call should be redirected to the equivalent deprecated
* method. <i>FOR INTERNAL USE ONLY. This method can be removed or renamed in future ASM
* versions</i>.
*
* @return returns true if {@link #preventRedirect()} has never been called, or if this is the
* first call to this method since the last call to {@link #preventRedirect()} (all this being
* restricted to the current thread only).
*/
protected static boolean shouldRedirect() {
boolean shouldRedirect = PREVENT_REDIRECT.get() != Boolean.TRUE;
PREVENT_REDIRECT.set(Boolean.FALSE);
return shouldRedirect;
}
// -----------------------------------------------------------------------------------------------