BytecodeAnalysis: force purity of enum.ordinal() and array.clone()

This commit is contained in:
Tagir Valeev
2018-01-15 16:29:57 +07:00
parent 9a4239100b
commit b414748d24
10 changed files with 90 additions and 8 deletions
@@ -53,7 +53,7 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension<HMethod> {
private static final ID<HMethod, Void> NAME = ID.create("bytecodeAnalysis");
private static final HKeyDescriptor KEY_DESCRIPTOR = new HKeyDescriptor();
private static final int VERSION = 7; // change when inference algorithm changes
private static final int VERSION = 8; // change when inference algorithm changes
private static final int VERSION_MODIFIER = HardCodedPurity.AGGRESSIVE_HARDCODED_PURITY ? 1 : 0;
private static final int FINAL_VERSION = VERSION * 2 + VERSION_MODIFIER;
@@ -175,6 +175,15 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator<Map<HMet
final PResults.PResult[] sharedResults = new PResults.PResult[Analysis.STEPS_LIMIT];
final Map<EKey, Equations> equations = new HashMap<>();
if ((classReader.getAccess() & Opcodes.ACC_ENUM) != 0) {
// ordinal() method is final in java.lang.Enum, but for some reason referred on call sites using specific enum class
// it's used on every enum switch statement, so forcing its purity is important
EKey ordinalKey = new EKey(new Method(classReader.getClassName(), "ordinal", "()I"), Out, true);
equations.put(ordinalKey, new Equations(
Collections.singletonList(new DirectionResultPair(Pure.asInt(), new Effects(DataValue.LocalDataValue, Collections.emptySet()))),
true));
}
classReader.accept(new KeyedMethodVisitor() {
protected MethodVisitor visitMethod(final MethodNode node, Method method, final EKey key) {
@@ -28,14 +28,14 @@ import java.util.Set;
class HardCodedPurity {
static final boolean AGGRESSIVE_HARDCODED_PURITY = Registry.is("java.annotations.inference.aggressive.hardcoded.purity", true);
private static Set<Couple<String>> ownedFields = ContainerUtil.set(
private static final Set<Couple<String>> ownedFields = ContainerUtil.set(
new Couple<>("java/lang/AbstractStringBuilder", "value")
);
private static Set<Method> thisChangingMethods = ContainerUtil.set(
private static final Set<Method> thisChangingMethods = ContainerUtil.set(
new Method("java/lang/Throwable", "fillInStackTrace", "()Ljava/lang/Throwable;")
);
// Assumed that all these methods are not only pure, but return object which could be safely modified
private static Set<Method> pureMethods = ContainerUtil.set(
private static final Set<Method> pureMethods = ContainerUtil.set(
// Maybe overloaded and be not pure, but this would be definitely bad code style
// Used in Throwable(Throwable) ctor, so this helps to infer purity of many exception constructors
new Method("java/lang/Throwable", "toString", "()Ljava/lang/String;"),
@@ -52,8 +52,8 @@ class HardCodedPurity {
new Method("java/lang/Double", "doubleToRawLongBits", "(D)J"),
new Method("java/lang/Double", "longBitsToDouble", "(J)D")
);
private static Map<Method, Set<EffectQuantum>> solutions = new HashMap<>();
private static Set<EffectQuantum> thisChange = Collections.singleton(EffectQuantum.ThisChangeQuantum);
private static final Map<Method, Set<EffectQuantum>> solutions = new HashMap<>();
private static final Set<EffectQuantum> thisChange = Collections.singleton(EffectQuantum.ThisChangeQuantum);
static {
// Native
@@ -92,7 +92,14 @@ class HardCodedPurity {
}
boolean isPureMethod(Method method) {
return pureMethods.contains(method);
if(pureMethods.contains(method)) {
return true;
}
// Array clone() method is a special beast: it's qualifier class is array itself
if(method.internalClassName.startsWith("[") && method.methodName.equals("clone") && method.methodDesc.equals("()Ljava/lang/Object;")) {
return true;
}
return false;
}
boolean isOwnedField(FieldInsnNode fieldInsn) {
@@ -453,6 +453,11 @@
<item name='java.io.ObjectInputStream java.lang.Enum readEnum(boolean)'>
<annotation name='org.jetbrains.annotations.Nullable'/>
</item>
<item name='java.io.ObjectInputStream java.lang.Object cloneArray(java.lang.Object)'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.io.ObjectInputStream java.lang.Object cloneArray(java.lang.Object) 0'>
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
@@ -1710,6 +1710,11 @@
<item name='java.lang.invoke.MethodType java.lang.Class&lt;?&gt;[] listToArray(java.util.List&lt;java.lang.Class&lt;?&gt;&gt;) 0'>
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
<item name='java.lang.invoke.MethodType java.lang.Class&lt;?&gt;[] parameterArray()'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.lang.invoke.MethodType java.lang.Class&lt;?&gt;[] ptypes()'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
@@ -1806,6 +1811,9 @@
</annotation>
</item>
<item name='java.lang.invoke.MethodType java.util.List&lt;java.lang.Class&lt;?&gt;&gt; parameterList()'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
<item name='java.lang.invoke.MethodType void MethodType_init(java.lang.Class&lt;?&gt;, java.lang.Class&lt;?&gt;[]) 0'>
@@ -1225,6 +1225,11 @@
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.Arrays.ArrayList java.lang.Object[] toArray()'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.Arrays.LegacyMergeSort LegacyMergeSort()'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
@@ -66,6 +66,11 @@
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.concurrent.atomic.AtomicIntegerArray AtomicIntegerArray(int[])'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.concurrent.atomic.AtomicIntegerArray AtomicIntegerArray(int[]) 0'>
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
@@ -179,6 +184,11 @@
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.concurrent.atomic.AtomicLongArray AtomicLongArray(long[])'>
<annotation name='org.jetbrains.annotations.Contract'>
<val name="pure" val="true"/>
</annotation>
</item>
<item name='java.util.concurrent.atomic.AtomicLongArray AtomicLongArray(long[]) 0'>
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
@@ -78,6 +78,7 @@ public class BytecodeAnalysisTest extends JavaCodeInsightFixtureTestCase {
checkAnnotations(Test02.class);
checkAnnotations(TestNonStable.class);
checkAnnotations(TestConflict.class);
checkAnnotations(TestEnum.class);
}
public void testHashCollision() {
@@ -142,7 +143,12 @@ public class BytecodeAnalysisTest extends JavaCodeInsightFixtureTestCase {
for (java.lang.reflect.Method javaMethod : javaClass.getDeclaredMethods()) {
if(javaMethod.isSynthetic()) continue; // skip lambda runtime representation
PsiMethod psiMethod = psiClass.findMethodsByName(javaMethod.getName(), false)[0];
PsiMethod psiMethod = ArrayUtil.getFirstElement(psiClass.findMethodsByName(javaMethod.getName(), false));
if (psiMethod == null) {
// Enum compilation adds some methods to bytecode which are not marked as synthetic
if(javaClass.isEnum()) continue;
fail("Unable to find method "+javaMethod.getName()+" in bytecode");
}
Annotation[][] annotations = javaMethod.getParameterAnnotations();
// not-null parameters
@@ -294,4 +294,11 @@ public class Test01 {
}
return false;
}
@ExpectContract(pure = true)
String[] replaceFirstWithNull(@ExpectNotNull String[] arr) {
String[] res = arr.clone();
res[0] = null;
return res;
}
}
@@ -0,0 +1,25 @@
/*
* Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
*/
package com.intellij.java.codeInspection.bytecodeAnalysis.data;
import com.intellij.java.codeInspection.bytecodeAnalysis.ExpectContract;
public enum TestEnum {
A, B, C;
@ExpectContract(pure = true)
public int getValue() {
return ordinal()+1;
}
@ExpectContract(pure = true)
public boolean isA() {
switch (this) {
case A:
return true;
default:
return false;
}
}
}