Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interning annotations #3

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion gson/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,44 @@
<name>Gson</name>

<dependencies>
<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker</artifactId>
<version>3.4.0</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.1</version>
<configuration>
<release>11</release>
<compilerArguments>
<Xmaxerrs>10000</Xmaxerrs>
<Xmaxwarns>10000</Xmaxwarns>
</compilerArguments>
<annotationProcessorPaths>
<path>
<groupId>org.checkerframework</groupId>
<artifactId>checker</artifactId>
<version>3.4.0</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<annotationProcessor>org.checkerframework.checker.interning.InterningChecker</annotationProcessor>
</annotationProcessors>
<compilerArgs>
<arg>-AprintErrorStack</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
Expand Down
4 changes: 4 additions & 0 deletions gson/src/main/java/com/google/gson/JsonArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ public boolean getAsBoolean() {
throw new IllegalStateException();
}

/*The equals function defined below is to compare two non-interned object
and only one of the condition to determine they are equal is a reference
equality test therefore its usage is safe*/
@SuppressWarnings("not.interned")
@Override
public boolean equals(Object o) {
return (o == this) || (o instanceof JsonArray && ((JsonArray) o).elements.equals(elements));
Expand Down
4 changes: 4 additions & 0 deletions gson/src/main/java/com/google/gson/JsonNull.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public int hashCode() {
/**
* All instances of JsonNull are the same
*/
/*The equals function defined below is to compare two non-interned object
and only one of the condition to determine they are equal is a reference
equality test therefore its usage is safe*/
@SuppressWarnings("not.interned")
@Override
public boolean equals(Object other) {
return this == other || other instanceof JsonNull;
Expand Down
4 changes: 4 additions & 0 deletions gson/src/main/java/com/google/gson/JsonObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ public JsonObject getAsJsonObject(String memberName) {
return (JsonObject) members.get(memberName);
}

/*The equals function defined below is to compare two non-interned object
and only one of the condition to determine they are equal is a reference
equality test therefore its usage is safe*/
@SuppressWarnings("not.interned")
@Override
public boolean equals(Object o) {
return (o == this) || (o instanceof JsonObject
Expand Down
3 changes: 2 additions & 1 deletion gson/src/main/java/com/google/gson/TypeAdapterFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import com.google.gson.reflect.TypeToken;
import org.checkerframework.checker.interning.qual.UsesObjectEquals;

/**
* Creates type adapters for set of related types. Type adapter factories are
Expand Down Expand Up @@ -160,7 +161,7 @@
*
* @since 2.1
*/
public interface TypeAdapterFactory {
public @UsesObjectEquals interface TypeAdapterFactory {

/**
* Returns a type adapter for {@code type}, or null if this factory doesn't
Expand Down
21 changes: 14 additions & 7 deletions gson/src/main/java/com/google/gson/internal/$Gson$Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ static boolean equal(Object a, Object b) {
/**
* Returns true if {@code a} and {@code b} are equal.
*/
/*The equals function defined below is to compare two non-interned object
and only one of the condition to determine they are equal is a reference
equality test therefore its usage is safe*/
@SuppressWarnings("not.interned")
public static boolean equals(Type a, Type b) {
if (a == b) {
// also handles (a == null && b == null)
Expand Down Expand Up @@ -337,6 +341,9 @@ public static Type resolve(Type context, Class<?> contextRawType, Type toResolve
return resolve(context, contextRawType, toResolve, new HashSet<TypeVariable>());
}

/*#1-#7 equality checks are reference equality check and not value
* equality check, therefore usage of == is safe*/
@SuppressWarnings("not.interned")
private static Type resolve(Type context, Class<?> contextRawType, Type toResolve,
Collection<TypeVariable> visitedTypeVariables) {
// this implementation is made a little more complicated in an attempt to avoid object-creation
Expand All @@ -350,36 +357,36 @@ private static Type resolve(Type context, Class<?> contextRawType, Type toResolv
visitedTypeVariables.add(typeVariable);
}
toResolve = resolveTypeVariable(context, contextRawType, typeVariable);
if (toResolve == typeVariable) {
if (toResolve == typeVariable) { //#1
return toResolve;
}

} else if (toResolve instanceof Class && ((Class<?>) toResolve).isArray()) {
Class<?> original = (Class<?>) toResolve;
Type componentType = original.getComponentType();
Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables);
return componentType == newComponentType
return componentType == newComponentType //#2
? original
: arrayOf(newComponentType);

} else if (toResolve instanceof GenericArrayType) {
GenericArrayType original = (GenericArrayType) toResolve;
Type componentType = original.getGenericComponentType();
Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables);
return componentType == newComponentType
return componentType == newComponentType //#3
? original
: arrayOf(newComponentType);

} else if (toResolve instanceof ParameterizedType) {
ParameterizedType original = (ParameterizedType) toResolve;
Type ownerType = original.getOwnerType();
Type newOwnerType = resolve(context, contextRawType, ownerType, visitedTypeVariables);
boolean changed = newOwnerType != ownerType;
boolean changed = newOwnerType != ownerType; //#4

Type[] args = original.getActualTypeArguments();
for (int t = 0, length = args.length; t < length; t++) {
Type resolvedTypeArgument = resolve(context, contextRawType, args[t], visitedTypeVariables);
if (resolvedTypeArgument != args[t]) {
if (resolvedTypeArgument != args[t]) { //#5
if (!changed) {
args = args.clone();
changed = true;
Expand All @@ -399,12 +406,12 @@ private static Type resolve(Type context, Class<?> contextRawType, Type toResolv

if (originalLowerBound.length == 1) {
Type lowerBound = resolve(context, contextRawType, originalLowerBound[0], visitedTypeVariables);
if (lowerBound != originalLowerBound[0]) {
if (lowerBound != originalLowerBound[0]) { //#6
return supertypeOf(lowerBound);
}
} else if (originalUpperBound.length == 1) {
Type upperBound = resolve(context, contextRawType, originalUpperBound[0], visitedTypeVariables);
if (upperBound != originalUpperBound[0]) {
if (upperBound != originalUpperBound[0]) { //#7
return subtypeOf(upperBound);
}
}
Expand Down
37 changes: 29 additions & 8 deletions gson/src/main/java/com/google/gson/internal/LinkedHashTreeMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.LinkedHashMap;
import java.util.NoSuchElementException;
import java.util.Set;
import org.checkerframework.checker.interning.qual.Interned;

/**
* A map of comparable keys to values. Unlike {@code TreeMap}, this class uses
Expand All @@ -38,14 +39,16 @@
* LinkedHashMap classes.
*/
public final class LinkedHashTreeMap<K, V> extends AbstractMap<K, V> implements Serializable {
@SuppressWarnings({ "unchecked", "rawtypes" }) // to avoid Comparable<Comparable<Comparable<...>>>
private static final Comparator<Comparable> NATURAL_ORDER = new Comparator<Comparable>() {
/*as the below Comparator implementing class definition creates an immutable
* object, therefore object created would be interned*/
@SuppressWarnings({ "unchecked", "rawtypes", "interned.object.creation" }) // to avoid Comparable<Comparable<Comparable<...>>>
private static @Interned final Comparator<Comparable> NATURAL_ORDER = new @Interned Comparator<Comparable>() {
public int compare(Comparable a, Comparable b) {
return a.compareTo(b);
}
};

Comparator<? super K> comparator;
@Interned Comparator<? super K> comparator;
Node<K, V>[] table;
final Node<K, V> header;
int size = 0;
Expand All @@ -69,7 +72,7 @@ public LinkedHashTreeMap() {
* use the natural ordering.
*/
@SuppressWarnings({ "unchecked", "rawtypes" }) // unsafe! if comparator is null, this assumes K is comparable
public LinkedHashTreeMap(Comparator<? super K> comparator) {
public LinkedHashTreeMap(@Interned Comparator<? super K> comparator) {
this.comparator = comparator != null
? comparator
: (Comparator) NATURAL_ORDER;
Expand Down Expand Up @@ -101,14 +104,19 @@ public LinkedHashTreeMap(Comparator<? super K> comparator) {
return result;
}

/*Here the usage of == to is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.
* It tries to search for a particular object or element in a tree and
* not a value therefore is safe #1*/
@SuppressWarnings("not.interned")
@Override public void clear() {
Arrays.fill(table, null);
size = 0;
modCount++;

// Clear all links to help GC
Node<K, V> header = this.header;
for (Node<K, V> e = header.next; e != header; ) {
for (Node<K, V> e = header.next; e != header; ) { //#1
Node<K, V> next = e.next;
e.next = e.prev = null;
e = next;
Expand Down Expand Up @@ -308,6 +316,11 @@ Node<K, V> removeInternalByKey(Object key) {
return node;
}

/*Here the usage of == to is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.
* It tries to search for a particular object or element in a tree and
* not a value therefore is safe #2 #3*/
@SuppressWarnings("not.interned")
private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
Node<K, V> parent = node.parent;
node.parent = null;
Expand All @@ -316,10 +329,10 @@ private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
}

if (parent != null) {
if (parent.left == node) {
if (parent.left == node) { //#2
parent.left = replacement;
} else {
assert (parent.right == node);
assert (parent.right == node); //#3
parent.right = replacement;
}
} else {
Expand Down Expand Up @@ -765,10 +778,18 @@ private abstract class LinkedTreeMapIterator<T> implements Iterator<T> {
LinkedTreeMapIterator() {
}

/*Here the usage of == to is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.#4
*/
@SuppressWarnings("not.interned")
public final boolean hasNext() {
return next != header;
return next != header; //#4
}

/*Here the usage of == to is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.#4
*/
@SuppressWarnings("not.interned")
final Node<K, V> nextNode() {
Node<K, V> e = next;
if (e == header) {
Expand Down
33 changes: 25 additions & 8 deletions gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.LinkedHashMap;
import java.util.NoSuchElementException;
import java.util.Set;
import org.checkerframework.checker.interning.qual.Interned;

/**
* A map of comparable keys to values. Unlike {@code TreeMap}, this class uses
Expand All @@ -36,14 +37,16 @@
* <p>This implementation was derived from Android 4.1's TreeMap class.
*/
public final class LinkedTreeMap<K, V> extends AbstractMap<K, V> implements Serializable {
@SuppressWarnings({ "unchecked", "rawtypes" }) // to avoid Comparable<Comparable<Comparable<...>>>
private static final Comparator<Comparable> NATURAL_ORDER = new Comparator<Comparable>() {
/*as the below Comparator implementing class definition creates an immutable
* object, therefore object created would be interned*/
@SuppressWarnings({ "unchecked", "rawtypes", "interned.object.creation" }) // to avoid Comparable<Comparable<Comparable<...>>>
private static final @Interned Comparator<Comparable> NATURAL_ORDER = new @Interned Comparator<Comparable>() {
public int compare(Comparable a, Comparable b) {
return a.compareTo(b);
}
};

Comparator<? super K> comparator;
@Interned Comparator<? super K> comparator;
Node<K, V> root;
int size = 0;
int modCount = 0;
Expand All @@ -68,7 +71,7 @@ public LinkedTreeMap() {
* use the natural ordering.
*/
@SuppressWarnings({ "unchecked", "rawtypes" }) // unsafe! if comparator is null, this assumes K is comparable
public LinkedTreeMap(Comparator<? super K> comparator) {
public LinkedTreeMap(@Interned Comparator<? super K> comparator) {
this.comparator = comparator != null
? comparator
: (Comparator) NATURAL_ORDER;
Expand Down Expand Up @@ -281,6 +284,11 @@ Node<K, V> removeInternalByKey(Object key) {
return node;
}

/*Here the usage of == is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.
* It tries to search for a particular object or element in a tree and
* not a value therefore is safe #1 #2*/
@SuppressWarnings("not.interned")
private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
Node<K, V> parent = node.parent;
node.parent = null;
Expand All @@ -289,10 +297,10 @@ private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
}

if (parent != null) {
if (parent.left == node) {
if (parent.left == node) { //#1
parent.left = replacement;
} else {
assert (parent.right == node);
assert (parent.right == node); //#2
parent.right = replacement;
}
} else {
Expand Down Expand Up @@ -531,13 +539,22 @@ private abstract class LinkedTreeMapIterator<T> implements Iterator<T> {
LinkedTreeMapIterator() {
}

/*Here the usage of == is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.#3
*/
@SuppressWarnings("not.interned")
public final boolean hasNext() {
return next != header;
return next != header; //#3
}

/*Here the usage of == is safe as the logic of the below function
* is concerned with the reference equality check and not value equality.
* It tries to search for a particular object or element in a tree and
* not a value therefore is safe #4*/
@SuppressWarnings("not.interned")
final Node<K, V> nextNode() {
Node<K, V> e = next;
if (e == header) {
if (e == header) { //#4
throw new NoSuchElementException();
}
if (modCount != expectedModCount) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ public JsonTreeReader(JsonElement element) {
return token != JsonToken.END_OBJECT && token != JsonToken.END_ARRAY;
}

/*Probable missing annotation in jdk, class Object should be annotated as
* UsesObjectEquals, therefore it is safe for objects of class Object to use
* reference equality check #1*/
@SuppressWarnings("not.interned")
@Override public JsonToken peek() throws IOException {
if (stackSize == 0) {
return JsonToken.END_DOCUMENT;
Expand Down Expand Up @@ -140,7 +144,7 @@ public JsonTreeReader(JsonElement element) {
}
} else if (o instanceof JsonNull) {
return JsonToken.NULL;
} else if (o == SENTINEL_CLOSED) {
} else if (o == SENTINEL_CLOSED) { //#1
throw new IllegalStateException("JsonReader is closed");
} else {
throw new AssertionError();
Expand Down
Loading