Skip to content

Commit

Permalink
Type-check tests for @GroupedList.Compressed.
Browse files Browse the repository at this point in the history
Each test enforces that all objects referenced by @GroupedList.Compressed in the package originate from one of the annotated static methods in GroupedList.

To be submitted after #8212.

PiperOrigin-RevId: 246052739
  • Loading branch information
Googler authored and copybara-github committed May 1, 2019
1 parent f2b386c commit baefeab
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/common/options",
"//third_party:apache_commons_lang",
"//third_party:checker_framework_annotations",
"//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:jsr305",
Expand Down
42 changes: 28 additions & 14 deletions src/main/java/com/google/devtools/build/lib/util/GroupedList.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import org.checkerframework.framework.qual.DefaultQualifierInHierarchy;
import org.checkerframework.framework.qual.ImplicitFor;
import org.checkerframework.framework.qual.LiteralKind;
import org.checkerframework.framework.qual.SubtypeOf;

/**
* Encapsulates a list of groups. Is intended to be used in "batch" mode -- to set the value of a
Expand All @@ -47,15 +51,17 @@ public class GroupedList<T> implements Iterable<List<T>> {
* Indicates that the annotated element is a compressed {@link GroupedList}, so that it can be
* safely passed to {@link #create} and friends.
*/
// TODO(jhorvitz): enforce this annotation via compile-time checks.
@Target({
ElementType.PARAMETER,
ElementType.FIELD,
ElementType.LOCAL_VARIABLE,
ElementType.TYPE_USE
})
@SubtypeOf(DefaultObject.class)
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
@ImplicitFor(literals = LiteralKind.NULL)
public @interface Compressed {}

/** Default annotation for type-safety checks of {@link Compressed}. */
@DefaultQualifierInHierarchy
@SubtypeOf({})
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
private @interface DefaultObject {}

// Total number of items in the list. At least elements.size(), but might be larger if there are
// any nested lists.
private int size = 0;
Expand Down Expand Up @@ -222,7 +228,7 @@ public int numElements() {
return size;
}

public static int numElements(Object compressed) {
public static int numElements(@Compressed Object compressed) {
if (compressed == EMPTY_LIST) {
return 0;
}
Expand Down Expand Up @@ -264,6 +270,16 @@ public static <T> Iterable<T> compressedToIterable(@Compressed Object compressed
return ImmutableList.of((T) compressed);
}

/**
* Casts an {@code Object} which is known to be {@link Compressed}.
*
* <p>This method should only be used when it is not possible to enforce the type via annotations.
*/
public static @Compressed Object castAsCompressed(Object obj) {
Preconditions.checkArgument(!(obj instanceof GroupedList), obj);
return (@Compressed Object) obj;
}

/** Returns true if this list contains no elements. */
public boolean isEmpty() {
return elements.isEmpty();
Expand Down Expand Up @@ -334,7 +350,7 @@ public static <E> GroupedList<E> create(@Compressed Object compressed) {
}

/** Creates an already compressed {@code GroupedList} for storage. */
public static <E> @Compressed Object createCompressedWithTwoGroupes(
public static <E> @Compressed Object createCompressedWithTwoGroups(
E singletonElementOfFirstGroup, List<? extends E> elementsOfSecondGroup) {
switch (elementsOfSecondGroup.size()) {
case 0:
Expand Down Expand Up @@ -441,10 +457,7 @@ public boolean equals(Object other) {

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("elements", elements)
.add("size", size).toString();

return MoreObjects.toStringHelper(this).add("elements", elements).add("size", size).toString();
}

/**
Expand Down Expand Up @@ -591,7 +604,8 @@ public String toString() {
return MoreObjects.toStringHelper(this)
.add("groupedList", groupedList)
.add("elements", elements)
.add("currentGroup", currentGroup).toString();
.add("currentGroup", currentGroup)
.toString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,8 @@ public class InMemoryNodeEntry implements NodeEntry {
*/
@Nullable protected volatile DirtyBuildingState dirtyBuildingState = null;

/**
* Construct a InMemoryNodeEntry. Use ONLY in Skyframe evaluation and graph implementations.
*/
public InMemoryNodeEntry() {
}
/** Construct a InMemoryNodeEntry. Use ONLY in Skyframe evaluation and graph implementations. */
public InMemoryNodeEntry() {}

// Public only for use in alternate graph implementations.
public KeepEdgesPolicy keepEdges() {
Expand Down Expand Up @@ -218,12 +215,12 @@ public synchronized Iterable<SkyKey> getDirectDeps() {
public synchronized @GroupedList.Compressed Object getCompressedDirectDepsForDoneEntry() {
assertKeepDeps();
Preconditions.checkState(isDone(), "no deps until done. NodeEntry: %s", this);
return Preconditions.checkNotNull(directDeps, "deps can't be null: %s", this);
Preconditions.checkNotNull(directDeps, "deps can't be null: %s", this);
return GroupedList.castAsCompressed(directDeps);
}

public int getNumDirectDeps() {
Preconditions.checkState(isDone(), "no deps until done. NodeEntry: %s", this);
return GroupedList.numElements(directDeps);
return GroupedList.numElements(getCompressedDirectDepsForDoneEntry());
}

@Override
Expand Down Expand Up @@ -520,7 +517,8 @@ public synchronized MarkedDirtyResult markDirty(DirtyType dirtyType) {
assertKeepDeps();
if (isDone()) {
dirtyBuildingState =
DirtyBuildingState.create(dirtyType, GroupedList.create(directDeps), value);
DirtyBuildingState.create(
dirtyType, GroupedList.create(getCompressedDirectDepsForDoneEntry()), value);
value = null;
directDeps = null;
return new MarkedDirtyResult(ReverseDepsUtility.getReverseDeps(this));
Expand Down Expand Up @@ -733,7 +731,7 @@ protected synchronized MoreObjects.ToStringHelper toStringHelper() {
.add(
"directDeps",
isDone() && keepEdges() != KeepEdgesPolicy.NONE
? GroupedList.create(directDeps)
? GroupedList.create(getCompressedDirectDepsForDoneEntry())
: directDeps)
.add("reverseDeps", ReverseDepsUtility.toString(this))
.add("dirtyBuildingState", dirtyBuildingState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,10 @@ public void createCompressed() {
GroupedList<String> groupedList = new GroupedList<>();
groupedList.appendGroup(ImmutableList.of("a"));
groupedList.appendGroup(ImmutableList.of("b", "c"));
assertThat(GroupedList.createCompressedWithTwoGroupes("a", ImmutableList.of("b", "c")))
assertThat(GroupedList.createCompressedWithTwoGroups("a", ImmutableList.of("b", "c")))
.isEqualTo(groupedList.compress());
groupedList.remove(ImmutableSet.of("b"));
assertThat(GroupedList.createCompressedWithTwoGroupes("a", ImmutableList.of("c")))
assertThat(GroupedList.createCompressedWithTwoGroups("a", ImmutableList.of("c")))
.isEqualTo(groupedList.compress());
}

Expand Down

0 comments on commit baefeab

Please sign in to comment.