bazel: encapsulate the representation of NestedSet
Before, NestedSet exposed its representation far and wide
in ways obvious (getChildrenUnsafe) and subtle (downcasting
NestedSetView.identity()).
This change encapsulates the representation of NestedSet,
allowing it to be improved in future. It also documents the
data type's conceptual model.
The API has changed as follows:
- NestedSetView is deleted. Its split, getDirect, and getTransitive
methods have been moved to NestedSet itself, since they have
always been well defined by the core abstraction.
getDirect is now getLeaves and getTransitive is getNonLeaves.
- NestedSetView.identifier is gone. Instead, NestedSet.Node is a
truly opaque identifier for a node in the graph's internal
representation. It provides only the hash and equals operations.
(It might seem more logical for getLeaves/NonLeaves to be defined
over Nodes, returning Nodes, but in practice no client seems
to need that.)
- NestedSet.getChildrenUnsafe is replaced by forEachElement,
which provides the pruned traversal needed by [redacted;
see b/157992832]. Unfortunately it still
exposes the internal node to the 'prune' callback, violating
encapsulation, though it is much less invasive than before.
Functionally, this method could be completely replaced
by NestedSetVisitor or by use of Node, but performance may
be a concern. Will address in a follow-up.
- Delete childrenToString.
- The public API no longer mentions "children".
Also, delete rawChildren() and expose NestedSet.children to the package.
This is a preparatory cleanup for CL 310551947, which will
represent the depth of the graph in the graph, obviating the
problem of set flattening throwing unchecked exceptions at
surprising times, the catching of which creates a bad dependency
from the Starlark interpreter to NestedSet.
PiperOrigin-RevId: 315079199
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
index 12e29b7..de31ed7 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
@@ -67,7 +67,6 @@
import com.google.devtools.build.lib.buildtool.buildevent.NoAnalyzeEvent;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Spawn;
import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
@@ -278,10 +277,7 @@
BuildEventStreamProtos.NamedSetOfFiles.Builder builder =
BuildEventStreamProtos.NamedSetOfFiles.newBuilder();
for (NestedSet<Artifact> artifactset : artifacts) {
- builder.addFileSets(
- converters
- .artifactGroupNamer()
- .apply((new NestedSetView<Artifact>(artifactset)).identifier()));
+ builder.addFileSets(converters.artifactGroupNamer().apply(artifactset.toNode()));
}
return GenericBuildEvent.protoChaining(this).setNamedSetOfFiles(builder.build()).build();
}
