Skip to content

Commit

Permalink
Give Children().empty() idiom a name: is_leaf().
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 334380631
  • Loading branch information
hzeller committed Sep 29, 2020
1 parent e57f68b commit 051c8ac
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 35 deletions.
2 changes: 1 addition & 1 deletion common/formatting/token_partition_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ std::vector<const UnwrappedLine*> FindLargestPartitions(
using partition_set_type = verible::TopN<const UnwrappedLine*, SizeCompare>;
partition_set_type partitions(num_partitions);
token_partitions.ApplyPreOrder([&partitions](const TokenPartitionTree& node) {
if (node.Children().empty()) { // only look at leaf partitions
if (node.is_leaf()) { // only look at leaf partitions
partitions.push(&node.Value());
}
});
Expand Down
4 changes: 2 additions & 2 deletions common/formatting/token_partition_tree_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ TEST_F(MergeLeafIntoPreviousLeafTest, OneChild) {
tree_type tree{
all, tree_type{all}, // subtree spans same range
};
ASSERT_FALSE(tree.Children().empty());
ASSERT_FALSE(tree.is_leaf());

const auto saved_tree(tree); // deep copy
auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children().front());
Expand Down Expand Up @@ -650,7 +650,7 @@ TEST_F(MergeLeafIntoNextLeafTest, OneChild) {
tree_type tree{
all, tree_type{all}, // subtree spans same range
};
ASSERT_FALSE(tree.Children().empty());
ASSERT_FALSE(tree.is_leaf());

const auto saved_tree(tree); // deep copy
auto* parent = MergeLeafIntoNextLeaf(&tree.Children().front());
Expand Down
4 changes: 2 additions & 2 deletions common/formatting/tree_unwrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ std::vector<UnwrappedLine> TreeUnwrapper::FullyPartitionedUnwrappedLines()
// visit only the node's children.
std::vector<UnwrappedLine> result;
unwrapped_lines_.ApplyPostOrder([&result](const TokenPartitionTree& node) {
if (node.Children().empty()) {
if (node.is_leaf()) {
result.push_back(node.Value());
}
});
Expand Down Expand Up @@ -191,7 +191,7 @@ void TreeUnwrapper::StartNewUnwrappedLine(PartitionPolicyEnum partitioning,
// for the sake of being able to correctly indent comments inside blocks.
// If so, delete those so that token partition range invariants are
// maintained through re-use of an existing node.
if (!active_unwrapped_lines_->Children().empty()) {
if (!active_unwrapped_lines_->is_leaf()) {
VLOG(4) << "removed pre-existing child partitions.";
active_unwrapped_lines_->Children().clear();
}
Expand Down
2 changes: 1 addition & 1 deletion common/util/expandable_tree_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class ExpandableTreeView {
// termination condition is different.
static const impl_type* first_unexpanded_child(const impl_type& current) {
const auto& info = current.Value();
if (info.IsExpanded() && !current.Children().empty()) {
if (info.IsExpanded() && !current.is_leaf()) {
// Let compiler to tail-call optimize self-recursion.
return first_unexpanded_child(current.Children().front());
} else {
Expand Down
8 changes: 5 additions & 3 deletions common/util/vector_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class _VectorTreeImpl {
// Returns the node reached by descending through Children().front().
template <typename TP>
static TP* _LeftmostDescendant(TP* node) {
while (!node->Children().empty()) {
while (!node->is_leaf()) {
node = &node->Children().front();
}
return node;
Expand All @@ -100,7 +100,7 @@ class _VectorTreeImpl {
// Returns the node reached by descending through Children().back().
template <typename TP>
static TP* _RightmostDescendant(TP* node) {
while (!node->Children().empty()) {
while (!node->is_leaf()) {
node = &node->Children().back();
}
return node;
Expand Down Expand Up @@ -377,6 +377,8 @@ class VectorTree : private _VectorTreeImpl {

const subnodes_type& Children() const { return children_; }

bool is_leaf() const { return children_.empty(); }

// Properties

// Returns the index of this node relative to parent's children.
Expand Down Expand Up @@ -736,7 +738,7 @@ class VectorTree : private _VectorTreeImpl {

// Concatenate children-without-grandchildren and grandchildren.
for (auto& child : temp) {
if (child.Children().empty()) {
if (child.is_leaf()) {
children_.emplace_back(std::move(child));
} else {
AdoptSubtreesFromUnreserved(&child.Children());
Expand Down
38 changes: 19 additions & 19 deletions common/util/vector_tree_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void ExpectPath(const Tree& tree, absl::string_view expect) {
// Test that basic Tree construction works on a singleton node.
TEST(VectorTreeTest, RootOnly) {
const VectorTreeTestType tree(verible::testing::MakeRootOnlyExampleTree());
EXPECT_TRUE(tree.Children().empty());
EXPECT_TRUE(tree.is_leaf());
EXPECT_EQ(tree.Parent(), nullptr);
EXPECT_EQ(tree.NumAncestors(), 0);
EXPECT_EQ(tree.BirthRank(), 0); // no parent
Expand Down Expand Up @@ -357,7 +357,7 @@ bool operator==(const NamedInterval& left, const NameOnly& right) {
TEST(VectorTreeTest, RootOnlyTreeTransformConstruction) {
const VectorTreeTestType tree(verible::testing::MakeRootOnlyExampleTree());
const auto other_tree = tree.Transform<NameOnly>(NameOnlyConverter);
EXPECT_TRUE(other_tree.Children().empty());
EXPECT_TRUE(other_tree.is_leaf());
EXPECT_EQ(other_tree.Parent(), nullptr);
EXPECT_EQ(other_tree.NumAncestors(), 0);
EXPECT_EQ(other_tree.BirthRank(), 0); // no parent
Expand Down Expand Up @@ -410,7 +410,7 @@ TEST(VectorTreeTest, NewChild) {
auto* child = tree.NewChild(NamedInterval(1, 2, "child"));
EXPECT_EQ(child->Parent(), &tree);
EXPECT_EQ(child->Root(), &tree);
EXPECT_TRUE(child->Children().empty());
EXPECT_TRUE(child->is_leaf());

const auto& value(child->Value());
EXPECT_EQ(value.left, 1);
Expand All @@ -423,7 +423,7 @@ TEST(VectorTreeTest, NewChild) {
auto* child = tree.NewChild(NamedInterval(2, 3, "lil-bro"));
EXPECT_EQ(child->Parent(), &tree);
EXPECT_EQ(child->Root(), &tree);
EXPECT_TRUE(child->Children().empty());
EXPECT_TRUE(child->is_leaf());

const auto& value(child->Value());
EXPECT_EQ(value.left, 2);
Expand All @@ -446,7 +446,7 @@ TEST(VectorTreeTest, NewSibling) {
// Recall that NewSibling() may invalidate reference to first_child.
EXPECT_EQ(second_child->Parent(), &tree);
EXPECT_EQ(second_child->Root(), &tree);
EXPECT_TRUE(second_child->Children().empty());
EXPECT_TRUE(second_child->is_leaf());

const auto& value(second_child->Value());
EXPECT_EQ(value.left, 2);
Expand All @@ -460,7 +460,7 @@ TEST(VectorTreeTest, NewSibling) {
TEST(VectorTreeTest, OneChildPolicy) {
const auto tree = verible::testing::MakeOneChildPolicyExampleTree();
EXPECT_EQ(tree.Parent(), nullptr);
EXPECT_FALSE(tree.Children().empty());
EXPECT_FALSE(tree.is_leaf());

const auto& value = tree.Value();
EXPECT_EQ(value.left, 0);
Expand All @@ -471,7 +471,7 @@ TEST(VectorTreeTest, OneChildPolicy) {
const auto& child = tree.Children().front();
EXPECT_EQ(child.Parent(), &tree);
EXPECT_EQ(child.Root(), &tree);
EXPECT_FALSE(child.Children().empty());
EXPECT_FALSE(child.is_leaf());
EXPECT_EQ(child.NumAncestors(), 1);
EXPECT_EQ(child.BirthRank(), 0);
EXPECT_TRUE(child.IsFirstChild());
Expand All @@ -494,7 +494,7 @@ TEST(VectorTreeTest, OneChildPolicy) {
const auto& grandchild = child.Children().front();
EXPECT_EQ(grandchild.Parent(), &child);
EXPECT_EQ(grandchild.Root(), &tree);
EXPECT_TRUE(grandchild.Children().empty());
EXPECT_TRUE(grandchild.is_leaf());
EXPECT_EQ(grandchild.NumAncestors(), 2);
EXPECT_EQ(grandchild.BirthRank(), 0);
EXPECT_TRUE(grandchild.IsFirstChild());
Expand Down Expand Up @@ -622,7 +622,7 @@ template <typename T>
void VerifyFamilyTree(const VectorTree<T>& tree) {
EXPECT_EQ(tree.Parent(), nullptr);
EXPECT_EQ(tree.Root(), &tree);
EXPECT_FALSE(tree.Children().empty());
EXPECT_FALSE(tree.is_leaf());
EXPECT_EQ(tree.NumAncestors(), 0);
EXPECT_EQ(tree.BirthRank(), 0);

Expand All @@ -634,7 +634,7 @@ void VerifyFamilyTree(const VectorTree<T>& tree) {
const auto& child = tree.Children()[i];
EXPECT_EQ(child.Parent(), &tree);
EXPECT_EQ(child.Root(), &tree);
EXPECT_FALSE(child.Children().empty());
EXPECT_FALSE(child.is_leaf());
EXPECT_EQ(child.NumAncestors(), 1);
EXPECT_EQ(child.BirthRank(), i);
EXPECT_EQ(child.IsFirstChild(), i == 0);
Expand All @@ -648,7 +648,7 @@ void VerifyFamilyTree(const VectorTree<T>& tree) {
const auto& grandchild = child.Children()[j];
EXPECT_EQ(grandchild.Parent(), &child);
EXPECT_EQ(grandchild.Root(), &tree);
EXPECT_TRUE(grandchild.Children().empty());
EXPECT_TRUE(grandchild.is_leaf());
EXPECT_EQ(grandchild.NumAncestors(), 2);
EXPECT_EQ(grandchild.BirthRank(), j);
EXPECT_EQ(grandchild.IsFirstChild(), j == 0);
Expand Down Expand Up @@ -1271,7 +1271,7 @@ TEST(VectorTreeTest, HoistOnlyChildRootOnly) {
// No children, no change.
EXPECT_FALSE(tree.HoistOnlyChild());

EXPECT_TRUE(tree.Children().empty());
EXPECT_TRUE(tree.is_leaf());
EXPECT_EQ(tree.Parent(), nullptr);
EXPECT_EQ(tree.NumAncestors(), 0);
EXPECT_EQ(tree.BirthRank(), 0); // no parent
Expand All @@ -1293,7 +1293,7 @@ TEST(VectorTreeTest, HoistOnlyChildOneChildTreeGreatestAncestor) {
const auto& child = tree;
EXPECT_EQ(child.Parent(), nullptr);
EXPECT_EQ(child.Root(), &tree);
EXPECT_FALSE(child.Children().empty());
EXPECT_FALSE(child.is_leaf());
EXPECT_EQ(child.NumAncestors(), 0);
EXPECT_EQ(child.BirthRank(), 0);

Expand All @@ -1314,7 +1314,7 @@ TEST(VectorTreeTest, HoistOnlyChildOneChildTreeGreatestAncestor) {
const auto& grandchild = child.Children().front();
EXPECT_EQ(grandchild.Parent(), &child);
EXPECT_EQ(grandchild.Root(), &tree);
EXPECT_TRUE(grandchild.Children().empty());
EXPECT_TRUE(grandchild.is_leaf());
EXPECT_EQ(grandchild.NumAncestors(), 1);
EXPECT_EQ(grandchild.BirthRank(), 0);

Expand Down Expand Up @@ -1362,7 +1362,7 @@ TEST(VectorTreeTest, HoistOnlyChildOneChildTreeMiddleAncestor) {
const auto& grandchild = tree.Children().front();
EXPECT_EQ(grandchild.Parent(), &tree);
EXPECT_EQ(grandchild.Root(), &tree);
EXPECT_TRUE(grandchild.Children().empty());
EXPECT_TRUE(grandchild.is_leaf());
EXPECT_EQ(grandchild.NumAncestors(), 1);
EXPECT_EQ(grandchild.BirthRank(), 0);

Expand Down Expand Up @@ -1408,12 +1408,12 @@ static std::vector<typename T::value_type> NodeValues(const T& node) {
TEST(VectorTreeTest, AdoptSubtreesFromEmptyToEmpty) {
typedef VectorTree<int> tree_type;
tree_type tree1(1), tree2(2); // no subtrees
EXPECT_TRUE(tree1.Children().empty());
EXPECT_TRUE(tree2.Children().empty());
EXPECT_TRUE(tree1.is_leaf());
EXPECT_TRUE(tree2.is_leaf());

tree1.AdoptSubtreesFrom(&tree2);
EXPECT_TRUE(tree1.Children().empty());
EXPECT_TRUE(tree2.Children().empty());
EXPECT_TRUE(tree1.is_leaf());
EXPECT_TRUE(tree2.is_leaf());
}

TEST(VectorTreeTest, AdoptSubtreesFromEmptyToNonempty) {
Expand Down
5 changes: 2 additions & 3 deletions verilog/formatting/tree_unwrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1628,8 +1628,7 @@ void TreeUnwrapper::ReshapeTokenPartitions(
AttachTrailingSemicolonToPreviousPartition(&partition);
// RHS may have been further partitioned, e.g. a macro call.
auto& children = partition.Children();
if (children.size() == 2 &&
children.front().Children().empty() /* left side */) {
if (children.size() == 2 && children.front().is_leaf() /* left side */) {
verible::MergeLeafIntoNextLeaf(&children.front());
VLOG(4) << "after merge leaf (left-into-right):\n" << partition;
}
Expand Down Expand Up @@ -1708,7 +1707,7 @@ void TreeUnwrapper::ReshapeTokenPartitions(
VLOG(4) << "block partition: " << seq_block_partition;
auto& begin_partition = *seq_block_partition.LeftmostDescendant();
VLOG(4) << "begin partition: " << begin_partition;
CHECK(begin_partition.Children().empty());
CHECK(begin_partition.is_leaf());
verible::MergeLeafIntoPreviousLeaf(&begin_partition);
VLOG(4) << "after merging 'begin' to predecessor:\n" << partition;
// Flatten only the statement block so that the control partition
Expand Down
2 changes: 1 addition & 1 deletion verilog/formatting/tree_unwrapper_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ typedef verible::VectorTree<ExpectedUnwrappedLine> ExpectedUnwrappedLineTree;
void ValidateExpectedTreeNode(const ExpectedUnwrappedLineTree& etree) {
// At each tree node, there should either be expected tokens in the node's
// value, or node's children, but not both.
CHECK(etree.Value().tokens.empty() != etree.Children().empty())
CHECK(etree.Value().tokens.empty() != etree.is_leaf())
<< "Node should not contain both tokens and children @"
<< verible::NodePath(etree);
}
Expand Down
3 changes: 1 addition & 2 deletions verilog/tools/kythe/indexing_facts_tree_extractor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,7 @@ void IndexingFactsTreeExtractor::ExtractMethodCallExtension(
IndexingNodeData function_node_data(IndexingFactType::kFunctionCall);
IndexingFactNode function_node(function_node_data);

if (facts_tree_context_.empty() ||
facts_tree_context_.top().Children().empty()) {
if (facts_tree_context_.empty() || facts_tree_context_.top().is_leaf()) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion verilog/tools/kythe/kythe_facts_extractor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ void KytheFactsExtractor::ExtractModuleNamedPort(
const VName port_vname_anchor = PrintAnchorVName(port_name);
CreateEdge(port_vname_anchor, kEdgeRef, *actual_port_vname);

if (named_port_node.Children().empty()) {
if (named_port_node.is_leaf()) {
const VName* definition_vname =
vertical_scope_resolver_.SearchForDefinition(port_name.Value());

Expand Down

0 comments on commit 051c8ac

Please sign in to comment.