From 7d77d455aa923d1376bf89688222339fcf52d09b Mon Sep 17 00:00:00 2001 From: Mariusz Glebocki Date: Sat, 30 Apr 2022 04:51:45 +0200 Subject: [PATCH] [WIP] Convert TokenPartitionTree into class hierarchy wrapper. TODO: - Extract Choice-related class and code to a separate commit - Documentation - Tests - Cleanup --- common/formatting/BUILD | 1 + common/formatting/align.cc | 28 +- common/formatting/align_test.cc | 30 +- common/formatting/layout_optimizer.cc | 58 +- common/formatting/layout_optimizer_test.cc | 60 +- common/formatting/token_partition_tree.cc | 166 +++-- common/formatting/token_partition_tree.h | 605 +++++++++++++++++- .../formatting/token_partition_tree_test.cc | 472 +++++++++----- .../token_partition_tree_test_utils.cc | 8 +- common/formatting/tree_unwrapper.cc | 16 +- common/util/tree_operations_test.cc | 8 +- verilog/formatting/tree_unwrapper.cc | 196 +++--- verilog/formatting/tree_unwrapper_test.cc | 2 +- 13 files changed, 1222 insertions(+), 428 deletions(-) diff --git a/common/formatting/BUILD b/common/formatting/BUILD index 30f32ab89c..34aaeef1d6 100644 --- a/common/formatting/BUILD +++ b/common/formatting/BUILD @@ -173,6 +173,7 @@ cc_library( ":format_token", ":line_wrap_searcher", ":unwrapped_line", + "//common/util:variant", "//common/strings:display_utils", "//common/strings:position", "//common/strings:range", diff --git a/common/formatting/align.cc b/common/formatting/align.cc index 1740db1960..1aec564469 100644 --- a/common/formatting/align.cc +++ b/common/formatting/align.cc @@ -983,7 +983,7 @@ void AlignablePartitionGroup::ApplyAlignment( const GroupAlignmentData& align_data) const { auto row = alignable_rows_.begin(); for (const auto& align_actions : align_data.align_actions_2D) { - (*row)->Children().clear(); + (*row)->Children()->clear(); VLOG(3) << __FUNCTION__ << " processing row: " << **row; if (!align_actions.empty()) { auto& node = **row; @@ -994,9 +994,9 @@ void AlignablePartitionGroup::ApplyAlignment( verible::TokenPartitionTree* current_cell = nullptr; if (align_actions.front().ftoken != ftokens.begin()) { - node.Children().emplace_back( + node.Children()->emplace_back( UnwrappedLine(0, ftokens.begin(), PartitionPolicyEnum::kInline)); - current_cell = &node.Children().back(); + current_cell = &node.Children()->back(); } for (const auto& action : align_actions) { @@ -1007,10 +1007,10 @@ void AlignablePartitionGroup::ApplyAlignment( << StringSpanOfTokenRange(current_cell->Value().TokensRange()) << " ]"; } - node.Children().emplace_back( + node.Children()->emplace_back( UnwrappedLine(action.new_before_spacing, action.ftoken, PartitionPolicyEnum::kInline)); - current_cell = &node.Children().back(); + current_cell = &node.Children()->back(); } if (current_cell) { current_cell->Value().SpanUpToToken(ftokens.end()); @@ -1140,7 +1140,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) { VLOG(4) << "partition before:\n" << TokenPartitionTreePrinter(partition, true); - partition.Children().clear(); + partition.Children()->clear(); auto tokens = partition.Value().TokensRange(); if (tokens.empty()) { partition.Value().SetPartitionPolicy( @@ -1157,7 +1157,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) { auto line = UnwrappedLine(indentation, tokens.begin(), PartitionPolicyEnum::kAlreadyFormatted); - partition.Children().emplace_back(line); + partition.Children()->emplace_back(line); if (tokens.size() > 1) { // First token @@ -1167,7 +1167,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) { auto slice = UnwrappedLine(0, tokens.begin(), PartitionPolicyEnum::kInline); slice.SpanNextToken(); - partition.Children().back().Children().emplace_back(slice); + partition.Children()->back().Children()->emplace_back(slice); // Remaining tokens for (auto it = tokens.begin() + 1; it != tokens.end(); ++it) { @@ -1180,7 +1180,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) { std::size_t last_newline_pos = whitespace.find_last_of('\n'); if (last_newline_pos != absl::string_view::npos) { // Update end of current line. - partition.Children().back().Value().SpanUpToToken(it); + partition.Children()->back().Value().SpanUpToToken(it); // Start a new line. // Newlines count does not matter here. All newlines in leading // whitespace of the first token in a line are always preserved. @@ -1192,19 +1192,19 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) { // indentation + (this line orig. indent) - (1st line orig. indent) const auto line = UnwrappedLine(0, it, PartitionPolicyEnum::kAlreadyFormatted); - partition.Children().emplace_back(line); + partition.Children()->emplace_back(line); // Count only spaces after the last '\n'. spacing -= last_newline_pos + 1; } auto slice = UnwrappedLine(spacing, it, PartitionPolicyEnum::kInline); slice.SpanNextToken(); - partition.Children().back().Children().emplace_back(slice); + partition.Children()->back().Children()->emplace_back(slice); } } - partition.Children().back().Value().SpanUpToToken(tokens.end()); + partition.Children()->back().Value().SpanUpToToken(tokens.end()); - if (partition.Children().size() == 1) { + if (partition.Children()->size() == 1) { HoistOnlyChild(partition); } else { partition.Value().SetPartitionPolicy(PartitionPolicyEnum::kAlwaysExpand); @@ -1271,7 +1271,7 @@ void TabularAlignTokens( // possibly some other ignored element like comments. auto& partition = *partition_ptr; - auto& subpartitions = partition.Children(); + auto& subpartitions = *partition.Children(); // Identify groups of partitions to align, separated by blank lines. const TokenPartitionRange subpartitions_range(subpartitions.begin(), subpartitions.end()); diff --git a/common/formatting/align_test.cc b/common/formatting/align_test.cc index f986ac7065..a400c0af69 100644 --- a/common/formatting/align_test.cc +++ b/common/formatting/align_test.cc @@ -162,7 +162,7 @@ class MatrixTreeAlignmentTestFixture : public AlignmentTestFixture { std::string Render() { std::ostringstream stream; - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { const auto policy = child.Value().PartitionPolicy(); if (policy == PartitionPolicyEnum::kAlreadyFormatted) { ApplyAlreadyFormattedPartitionPropertiesToTokens(&child, @@ -333,7 +333,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, OneInterTokenPaddingWithIndent) { ftoken.before.spaces_required = 1; } // Indent each partition. - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { child.Value().SetIndentationSpaces(4); } @@ -354,7 +354,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, IgnoreCommentLine) { } // Leave the 'commented' line indented. pre_format_tokens_[2].before.break_decision = SpacingOptions::MustWrap; - partition_.Children()[1].Value().SetIndentationSpaces(1); + partition_.Children()->at(1).Value().SetIndentationSpaces(1); // Pretend lines that begin with "three" are to be ignored, like comments. auto ignore_threes = [](const TokenPartitionTree& partition) { @@ -406,7 +406,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, CompletelyDisabledNoAlignmentWithIndent) { for (auto& ftoken : pre_format_tokens_) { ftoken.before.spaces_required = 1; } - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { child.Value().SetIndentationSpaces(3); } pre_format_tokens_[0].before.break_decision = SpacingOptions::MustWrap; @@ -442,7 +442,7 @@ TEST_F(Sparse3x3MatrixAlignmentMoreSpacesTest, for (auto& ftoken : pre_format_tokens_) { ftoken.before.spaces_required = 1; } - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { child.Value().SetIndentationSpaces(1); } pre_format_tokens_[0].before.break_decision = SpacingOptions::MustWrap; @@ -510,7 +510,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, DisabledByColumnLimitIndented) { for (auto& ftoken : pre_format_tokens_) { ftoken.before.spaces_required = 1; } - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { child.Value().SetIndentationSpaces(3); } @@ -592,7 +592,7 @@ class MultiAlignmentGroupTest : public AlignmentTestFixture { std::ostringstream stream; int position = 0; const absl::string_view text(sample_); - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { const auto policy = child.Value().PartitionPolicy(); if (policy == PartitionPolicyEnum::kAlreadyFormatted) { ApplyAlreadyFormattedPartitionPropertiesToTokens(&child, @@ -717,8 +717,8 @@ class GetPartitionAlignmentSubrangesTestFixture : public AlignmentTestFixture { }; TEST_F(GetPartitionAlignmentSubrangesTestFixture, VariousRanges) { - const TokenPartitionRange children(partition_.Children().begin(), - partition_.Children().end()); + const TokenPartitionRange children(partition_.Children()->begin(), + partition_.Children()->end()); const std::vector ranges( GetPartitionAlignmentSubranges(children, [](const TokenPartitionTree& @@ -820,8 +820,8 @@ class GetPartitionAlignmentSubrangesSubtypedTestFixture }; TEST_F(GetPartitionAlignmentSubrangesSubtypedTestFixture, VariousRanges) { - const TokenPartitionRange children(partition_.Children().begin(), - partition_.Children().end()); + const TokenPartitionRange children(partition_.Children()->begin(), + partition_.Children()->end()); const std::vector ranges( GetPartitionAlignmentSubranges(children, &PartitionSelector)); @@ -1058,7 +1058,7 @@ class SubcolumnsTreeAlignmentTest : public MatrixTreeAlignmentTestFixture { } uwline.SpanUpToToken(token_iter); uwline.SetOrigin(item.get()); - partition_.Children().emplace_back(std::move(uwline)); + partition_.Children()->emplace_back(std::move(uwline)); SymbolCastToNode(*syntax_tree_).AppendChild(std::move(item)); } } @@ -1185,7 +1185,7 @@ TEST_F(SubcolumnsTreeAlignmentTest, OneInterTokenPaddingExceptFront) { ftoken.before.spaces_required = 1; } // Find first token of each line and require 0 spaces before them. - for (auto& line : partition_.Children()) { + for (auto& line : *partition_.Children()) { const auto tokens = line.Value().TokensRange(); if (!tokens.empty()) { const PreFormatToken& front = tokens.front(); @@ -1232,7 +1232,7 @@ TEST_F(SubcolumnsTreeAlignmentTest, for (auto& ftoken : pre_format_tokens_) { ftoken.before.spaces_required = 1; } - for (auto& line : partition_.Children()) { + for (auto& line : *partition_.Children()) { line.Value().SetIndentationSpaces(2); } @@ -1262,7 +1262,7 @@ class MultiSubcolumnsTreeAlignmentTest : public SubcolumnsTreeAlignmentTest { std::ostringstream stream; int position = 0; const absl::string_view text(sample_); - for (auto& child : partition_.Children()) { + for (auto& child : *partition_.Children()) { const auto policy = child.Value().PartitionPolicy(); if (policy == PartitionPolicyEnum::kAlreadyFormatted) { ApplyAlreadyFormattedPartitionPropertiesToTokens(&child, diff --git a/common/formatting/layout_optimizer.cc b/common/formatting/layout_optimizer.cc index c8a549c449..4cd3f580d5 100644 --- a/common/formatting/layout_optimizer.cc +++ b/common/formatting/layout_optimizer.cc @@ -90,7 +90,7 @@ int AlreadyFormattedPartitionLength(const TokenPartitionTree& partition) { width += token.before.spaces_required + token.Length(); } - for (const auto& child : partition.Children()) { + for (const auto& child : *partition.Children()) { CHECK_EQ(child.Value().PartitionPolicy(), PartitionPolicyEnum::kInline); if (child.Value().TokensRange().begin() != tokens.begin()) { const auto& first_token = child.Value().TokensRange().front(); @@ -500,7 +500,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( // Traverse and calculate children layouts - absl::FixedArray layouts(node.Children().size()); + absl::FixedArray layouts(node.Children()->size()); switch (node.Value().PartitionPolicy()) { case PartitionPolicyEnum::kJuxtaposition: @@ -509,7 +509,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( case PartitionPolicyEnum::kFitOnLineElseExpand: case PartitionPolicyEnum::kAppendFittingSubPartitions: case PartitionPolicyEnum::kJuxtapositionOrIndentedStack: { - std::transform(node.Children().begin(), node.Children().end(), + std::transform(node.Children()->begin(), node.Children()->end(), layouts.begin(), [=](const TokenPartitionTree& n) { return this->CalculateOptimalLayout(n); }); @@ -520,7 +520,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( case PartitionPolicyEnum::kAlwaysExpand: case PartitionPolicyEnum::kTabularAlignment: { const int indentation = node.Value().IndentationSpaces(); - std::transform(node.Children().begin(), node.Children().end(), + std::transform(node.Children()->begin(), node.Children()->end(), layouts.begin(), [=](const TokenPartitionTree& n) { const int relative_indentation = n.Value().IndentationSpaces() - indentation; @@ -558,10 +558,11 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( case PartitionPolicyEnum::kStack: return factory_.Stack(layouts.begin(), layouts.end()); case PartitionPolicyEnum::kWrap: { - if (VLOG_IS_ON(0) && node.Children().size() > 2) { - const int indentation = node.Children()[1].Value().IndentationSpaces(); - for (const auto& child : iterator_range(node.Children().begin() + 2, - node.Children().end())) { + if (VLOG_IS_ON(0) && node.Children()->size() > 2) { + const int indentation = + node.Children()->at(1).Value().IndentationSpaces(); + for (const auto& child : iterator_range(node.Children()->begin() + 2, + node.Children()->end())) { if (child.Value().IndentationSpaces() != indentation) { VLOG(0) << "Indentations of subpartitions from the second to the " "last are not equal. Using indentation of the second " @@ -571,8 +572,8 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( } } const int hanging_indentation = - (node.Children().size() > 1) - ? (node.Children()[1].Value().IndentationSpaces() - + (node.Children()->size() > 1) + ? (node.Children()->at(1).Value().IndentationSpaces() - node.Value().IndentationSpaces()) : 0; @@ -592,7 +593,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( const int indentation = node.Value().IndentationSpaces(); for (size_t i = 0; i < layouts.size(); ++i) { const int relative_indentation = - node.Children()[i].Value().IndentationSpaces() - indentation; + node.Children()->at(i).Value().IndentationSpaces() - indentation; layouts[i] = factory_.Indent(layouts[i], relative_indentation); } auto stack = factory_.Stack(layouts.begin(), layouts.end()); @@ -615,7 +616,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( // When not a leaf, it contains partitions with kInline // policy. Pack them horizontally. const bool all_children_are_inlines = - std::all_of(node.Children().begin(), node.Children().end(), + std::all_of(node.Children()->begin(), node.Children()->end(), [](const TokenPartitionTree& child) { return child.Value().PartitionPolicy() == PartitionPolicyEnum::kInline; @@ -631,7 +632,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout( // Preserve spacing of the first sublayout. This has to be done because // the first layout in a line uses IndentationSpaces instead of // SpacesBefore. - const auto indent = node.Children().front().Value().IndentationSpaces(); + const auto indent = node.Children()->front().Value().IndentationSpaces(); layouts.front() = factory_.Indent(layouts.front(), indent); return factory_.Juxtaposition(layouts.begin(), layouts.end()); @@ -679,26 +680,26 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) { layout.TokensRange().begin(), PartitionPolicyEnum::kAlreadyFormatted); uwline.SpanUpToToken(layout.TokensRange().end()); - tree_.Children().emplace_back(uwline); - current_node_ = &tree_.Children().back(); + tree_.Children()->emplace_back(uwline); + current_node_ = &tree_.Children()->back(); } else { const auto tokens = layout.TokensRange(); CHECK(current_node_->Value().TokensRange().end() == tokens.begin()); current_node_->Value().SpanUpToToken(tokens.end()); - auto& slices = current_node_->Children(); + auto& slices = *current_node_->Children(); // TODO(mglb): add support for break_decision == Preserve if (layout.SpacesBefore() == tokens.front().before.spaces_required) { // No need for separate inline partition - if (!slices.empty()) + if (!is_leaf(*current_node_)) slices.back().Value().SpanUpToToken(tokens.end()); return; } // Wrap previous tokens in the line - if (slices.empty()) { - current_node_->Children().emplace_back( + if (is_leaf(*current_node_)) { + current_node_->Children()->emplace_back( UnwrappedLine(0, current_node_->Value().TokensRange().begin(), PartitionPolicyEnum::kInline)); } @@ -708,7 +709,7 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) { auto slice = UnwrappedLine(layout.SpacesBefore(), tokens.begin(), PartitionPolicyEnum::kInline); slice.SpanUpToToken(tokens.end()); - current_node_->Children().emplace_back(slice); + current_node_->Children()->emplace_back(slice); } return; } @@ -756,20 +757,19 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) { void TreeReconstructor::ReplaceTokenPartitionTreeNode( TokenPartitionTree* node) { CHECK_NOTNULL(node); - CHECK(!tree_.Children().empty()); + CHECK(!is_leaf(tree_)); - if (tree_.Children().size() == 1) { - *node = std::move(tree_.Children().front()); + if (tree_.Children()->size() == 1) { + *node = std::move(tree_.Children()->front()); } else { - const auto& first_line = tree_.Children().front().Value(); - const auto& last_line = tree_.Children().back().Value(); + const auto& first_line = tree_.Children()->front().Value(); + const auto& last_line = tree_.Children()->back().Value(); - node->Value() = UnwrappedLine(current_indentation_spaces_, + tree_.Value() = UnwrappedLine(current_indentation_spaces_, first_line.TokensRange().begin(), PartitionPolicyEnum::kAlwaysExpand); - node->Value().SpanUpToToken(last_line.TokensRange().end()); - node->Children().clear(); - AdoptSubtreesFrom(*node, &tree_); + tree_.Value().SpanUpToToken(last_line.TokensRange().end()); + *node = std::move(tree_); } } diff --git a/common/formatting/layout_optimizer_test.cc b/common/formatting/layout_optimizer_test.cc index b957a34bb7..3419966804 100644 --- a/common/formatting/layout_optimizer_test.cc +++ b/common/formatting/layout_optimizer_test.cc @@ -2294,10 +2294,10 @@ TEST_F(TreeReconstructorTest, InlineSpacingReconstruction) { .build(pre_format_tokens_); const auto layout_tree = - LT(LI(LayoutType::kJuxtaposition, 1, true), // - LT(LI(tree_expected.Children()[0].Value(), true, 1)), // - LT(LI(tree_expected.Children()[1].Value())), // - LT(LI(tree_expected.Children()[2].Value()))); + LT(LI(LayoutType::kJuxtaposition, 1, true), // + LT(LI(tree_expected.Children()->at(0).Value(), true, 1)), // + LT(LI(tree_expected.Children()->at(1).Value())), // + LT(LI(tree_expected.Children()->at(2).Value()))); auto optimized_tree = TokenPartitionTree(); @@ -2435,10 +2435,10 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value())), // - LT(LI(tree.Children()[1].Value())), // - LT(LI(tree.Children()[2].Value()))); + const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value())), // + LT(LI(tree.Children()->at(1).Value())), // + LT(LI(tree.Children()->at(2).Value()))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 5, 4.0F, 0}, {35, expected_layout, 5, 4.0F, 100}, @@ -2457,10 +2457,11 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value(), 1)), // - LT(LI(tree.Children()[1].Value(), 2)), // - LT(LI(tree.Children()[2].Value(), 3))); + const auto expected_layout = + LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value(), 1)), // + LT(LI(tree.Children()->at(1).Value(), 2)), // + LT(LI(tree.Children()->at(2).Value(), 3))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 8, 4.0F, 0}, {32, expected_layout, 8, 4.0F, 100}, @@ -2480,10 +2481,10 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value())), // - LT(LI(tree.Children()[1].Value())), // - LT(LI(tree.Children()[2].Value()))); + const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value())), // + LT(LI(tree.Children()->at(1).Value())), // + LT(LI(tree.Children()->at(2).Value()))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 5, 4.0F, 0}, {35, expected_layout, 5, 4.0F, 100}, @@ -2502,10 +2503,11 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value(), 1)), // - LT(LI(tree.Children()[1].Value(), 2)), // - LT(LI(tree.Children()[2].Value(), 3))); + const auto expected_layout = + LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value(), 1)), // + LT(LI(tree.Children()->at(1).Value(), 2)), // + LT(LI(tree.Children()->at(2).Value(), 3))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 8, 4.0F, 0}, {32, expected_layout, 8, 4.0F, 100}, @@ -2524,9 +2526,9 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value())), // - LT(LI(tree.Children()[1].Value()))); + const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value())), // + LT(LI(tree.Children()->at(1).Value()))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 31, 2.0F, 0}, {9, expected_layout, 31, 2.0F, 100}, @@ -2544,9 +2546,9 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { }) .build(pre_format_tokens_); - const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // - LT(LI(tree.Children()[0].Value())), // - LT(LI(tree.Children()[1].Value()))); + const auto expected_layout = LT(LI(LayoutType::kStack, 0, true), // + LT(LI(tree.Children()->at(0).Value())), // + LT(LI(tree.Children()->at(1).Value()))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 31, 2.0F, 0}, {9, expected_layout, 31, 2.0F, 100}, @@ -2567,9 +2569,9 @@ TEST_F(TokenPartitionsLayoutOptimizerTest, CalculateOptimalLayout) { const auto expected_layout = LT(LI(LayoutType::kJuxtaposition, 3, true), - LT(LI(tree.Children()[0].Value(), true, 3)), - LT(LI(tree.Children()[1].Value(), false, 0)), - LT(LI(tree.Children()[2].Value(), false, 0))); + LT(LI(tree.Children()->at(0).Value(), true, 3)), + LT(LI(tree.Children()->at(1).Value(), false, 0)), + LT(LI(tree.Children()->at(2).Value(), false, 0))); const auto expected_lf = LayoutFunction{ {0, expected_layout, 31, 0.0, 0}, diff --git a/common/formatting/token_partition_tree.cc b/common/formatting/token_partition_tree.cc index 2a192c25c5..bcae801127 100644 --- a/common/formatting/token_partition_tree.cc +++ b/common/formatting/token_partition_tree.cc @@ -46,8 +46,8 @@ void VerifyTreeNodeFormatTokenRanges(const TokenPartitionTree& node, return std::distance(base, iter); }; - const auto& children = node.Children(); - if (!children.empty()) { + if (!is_leaf(node)) { + const auto& children = *node.Children(); const TokenPartitionTreePrinter node_printer(node); { // Hierarchy invariant: parent's range == range spanned by children. @@ -132,9 +132,11 @@ std::vector> FlushLeftSpacingDifferences( std::ostream& TokenPartitionTreePrinter::PrintTree(std::ostream& stream, int indent) const { const auto& value = node.Value(); - const auto& children = node.Children(); + const auto& children = *node.Children(); stream << Spacer(indent) << "{ "; - if (children.empty()) { + if ((node.kind() == TokenPartitionTree::Kind::kChoice && + node.Choices().empty()) || + (node.kind() != TokenPartitionTree::Kind::kChoice && children.empty())) { stream << '('; value.AsCode(&stream, verbose, origin_printer); stream << ") }"; @@ -142,23 +144,36 @@ std::ostream& TokenPartitionTreePrinter::PrintTree(std::ostream& stream, stream << '(' // similar to UnwrappedLine::AsCode() << Spacer(value.IndentationSpaces(), - UnwrappedLine::kIndentationMarker) - // just means the concatenation of all subpartitions - << "[], policy: " << value.PartitionPolicy() << ") @" - << NodePath(node); + UnwrappedLine::kIndentationMarker); + // just means the concatenation of all subpartitions + if (node.kind() == TokenPartitionTree::Kind::kChoice) { + stream << "[], ) @"; + } else { + stream << "[], policy: " << value.PartitionPolicy() << ") @"; + } + stream << NodePath(node); if (value.Origin() != nullptr) { stream << ", (origin: "; origin_printer(stream, value.Origin()); stream << ")"; } stream << '\n'; - // token range spans all of children nodes - for (const auto& child : children) { - TokenPartitionTreePrinter(child, verbose, origin_printer) - .PrintTree(stream, indent + 2) - << '\n'; + if (node.kind() == TokenPartitionTree::Kind::kChoice) { + for (const auto& child : node.Choices()) { + TokenPartitionTreePrinter(child, verbose, origin_printer) + .PrintTree(stream, indent + 2) + << '\n'; + } + stream << Spacer(indent) << '}'; + } else { + // token range spans all of children nodes + for (const auto& child : children) { + TokenPartitionTreePrinter(child, verbose, origin_printer) + .PrintTree(stream, indent + 2) + << '\n'; + } + stream << Spacer(indent) << '}'; } - stream << Spacer(indent) << '}'; } return stream; } @@ -263,9 +278,15 @@ bool AnyPartitionSubRangeIsDisabled(TokenPartitionRange range, } void AdjustIndentationRelative(TokenPartitionTree* tree, int amount) { - ApplyPreOrder(*ABSL_DIE_IF_NULL(tree), [&](UnwrappedLine& line) { - const int new_indent = std::max(line.IndentationSpaces() + amount, 0); - line.SetIndentationSpaces(new_indent); + ApplyPreOrder(*ABSL_DIE_IF_NULL(tree), [&](TokenPartitionTree& node) { + const int new_indent = + std::max(node.Value().IndentationSpaces() + amount, 0); + node.Value().SetIndentationSpaces(new_indent); + if (node.IsChoice()) { + for (auto& choice : node.Choices()) { + AdjustIndentationRelative(&choice, amount); + } + } }); } @@ -322,7 +343,7 @@ void ApplyAlreadyFormattedPartitionPropertiesToTokens( mutable_tokens_begin->before.break_decision = verible::SpacingOptions::MustWrap; - for (auto& child : already_formatted_partition_node->Children()) { + for (auto& child : *already_formatted_partition_node->Children()) { auto slice = child.Value(); if (slice.PartitionPolicy() != PartitionPolicyEnum::kInline) { VLOG(1) << "Partition policy is not kInline - ignoring. Parent " @@ -347,16 +368,18 @@ void ApplyAlreadyFormattedPartitionPropertiesToTokens( decision = verible::SpacingOptions::MustAppend; } // Children are no longer needed - already_formatted_partition_node->Children().clear(); + already_formatted_partition_node->Children()->clear(); VLOG(4) << __FUNCTION__ << ": partition after:\n" << TokenPartitionTreePrinter(*already_formatted_partition_node, true); } void MergeConsecutiveSiblings(TokenPartitionTree* tree, size_t pos) { CHECK_NOTNULL(tree); - CHECK_LT(pos + 1, tree->Children().size()); - const auto& current = tree->Children()[pos]; - const auto& next = tree->Children()[pos + 1]; + CHECK_LT(pos + 1, tree->Children()->size()); + const auto& current = tree->Children()->at(pos); + const auto& next = tree->Children()->at(pos + 1); + CHECK(current.IsPartition() && !current.IsAlreadyFormatted()); + CHECK(next.IsPartition() && !next.IsAlreadyFormatted()); // Merge of a non-leaf partition and a leaf partition produces a non-leaf // partition with token range wider than concatenated token ranges of its // children. @@ -411,18 +434,25 @@ TokenPartitionTree* GroupLeafWithPreviousLeaf(TokenPartitionTree* leaf) { auto* leaf_parent = leaf->Parent(); { - // Extend the upper-bound of the previous leaf partition to cover the - // partition that is about to be removed. - const auto range_end = leaf->Value().TokensRange().end(); const auto uwline = leaf->Value(); + const auto range_end = uwline.TokensRange().end(); const auto previous_uwline = previous_leaf->Value(); + + TokenPartitionTree group(previous_uwline); + group.Children()->reserve(2); + group.Children()->push_back(std::move(*previous_leaf)); + group.Children()->push_back(std::move(*leaf)); + + *previous_leaf = std::move(group); + // Extend the upper-bound of the group partition to cover the + // partition that is about to be removed. UpdateTokenRangeUpperBound(previous_leaf, &common_ancestor, range_end); - previous_leaf->Children().emplace_back(previous_uwline); - previous_leaf->Children().emplace_back(uwline); + if (range_end > common_ancestor.Value().TokensRange().end()) { common_ancestor.Value().SpanUpToToken(range_end); } VLOG(5) << "common ancestor (after updating target):\n" << common_ancestor; + // Shrink lower-bounds of the originating subtree. UpdateTokenRangeLowerBound(leaf_parent, &common_ancestor, range_end); VLOG(5) << "common ancestor (after updating origin):\n" << common_ancestor; @@ -442,13 +472,75 @@ TokenPartitionTree* GroupLeafWithPreviousLeaf(TokenPartitionTree* leaf) { return previous_leaf; } +TokenPartitionTree* GroupLeafWithNextLeaf(TokenPartitionTree* leaf) { + CHECK_NOTNULL(leaf); + VLOG(4) << "origin leaf:\n" << *leaf; + auto* next_leaf = NextLeaf(*leaf); + if (next_leaf == nullptr) return nullptr; + VLOG(4) << "next leaf:\n" << *next_leaf; + + // If there is no common ancestor, do nothing and return. + auto& common_ancestor = + *ABSL_DIE_IF_NULL(NearestCommonAncestor(*leaf, *next_leaf)); + VLOG(4) << "common ancestor:\n" << common_ancestor; + + // Verify continuity of token ranges between adjacent leaves. + CHECK(leaf->Value().TokensRange().end() == + next_leaf->Value().TokensRange().begin()); + + auto* leaf_parent = leaf->Parent(); + { + const auto uwline = leaf->Value(); + const auto range_begin = uwline.TokensRange().begin(); + const auto next_uwline = next_leaf->Value(); + + TokenPartitionTree group(next_uwline); + group.Children()->reserve(2); + group.Children()->push_back(std::move(*leaf)); + group.Children()->push_back(std::move(*next_leaf)); + + *next_leaf = std::move(group); + + UpdateTokenRangeLowerBound(next_leaf, &common_ancestor, range_begin); + if (range_begin < common_ancestor.Value().TokensRange().begin()) { + common_ancestor.Value().SpanBackToToken(range_begin); + } + VLOG(4) << "common ancestor (after updating target):\n" << common_ancestor; + + // Extend the upper-bound of the group partition to cover the + // partition that is about to be removed. + UpdateTokenRangeUpperBound(next_leaf, &common_ancestor, range_begin); + VLOG(4) << "common ancestor (after updating origin):\n" << common_ancestor; + + // Shrink upper-bounds of the originating subtree. + UpdateTokenRangeUpperBound(leaf_parent, &common_ancestor, range_begin); + + // Remove the obsolete partition, leaf. + // Caution: Existing references to the obsolete partition (and beyond) + // will be invalidated! + RemoveSelfFromParent(*leaf); + VLOG(4) << "common ancestor (after destroying leaf):\n" << common_ancestor; + } + + // Sanity check invariants. + VerifyFullTreeFormatTokenRanges( + common_ancestor, + LeftmostDescendant(common_ancestor).Value().TokensRange().begin()); + + return next_leaf; +} + // Note: this destroys leaf TokenPartitionTree* MergeLeafIntoPreviousLeaf(TokenPartitionTree* leaf) { + // TODO(mglb): find LeafPartition or Choice or AlreadyFormatted instead of + // leaf CHECK_NOTNULL(leaf); VLOG(4) << "origin leaf:\n" << *leaf; + CHECK(leaf->IsLeafPartition() && !leaf->IsAlreadyFormatted()); auto* target_leaf = PreviousLeaf(*leaf); if (target_leaf == nullptr) return nullptr; VLOG(4) << "target leaf:\n" << *target_leaf; + CHECK(target_leaf->IsLeafPartition() && !target_leaf->IsAlreadyFormatted()); // If there is no common ancestor, do nothing and return. auto& common_ancestor = @@ -492,9 +584,11 @@ TokenPartitionTree* MergeLeafIntoPreviousLeaf(TokenPartitionTree* leaf) { TokenPartitionTree* MergeLeafIntoNextLeaf(TokenPartitionTree* leaf) { CHECK_NOTNULL(leaf); VLOG(4) << "origin leaf:\n" << *leaf; + CHECK(leaf->IsLeafPartition() && !leaf->IsAlreadyFormatted()); auto* target_leaf = NextLeaf(*leaf); if (target_leaf == nullptr) return nullptr; VLOG(4) << "target leaf:\n" << *target_leaf; + CHECK(target_leaf->IsLeafPartition() && !target_leaf->IsAlreadyFormatted()); // If there is no common ancestor, do nothing and return. auto& common_ancestor = @@ -787,16 +881,16 @@ void ReshapeFittingSubpartitions(const BasicFormatStyle& style, VectorTree* fitted_tree = nullptr; // Leaf or simple node, e.g. '[function foo ( ) ;]' - if (node->Children().size() < 2) { + if (node->Children()->size() < 2) { // Nothing to do return; } // Partition with arguments should have at least one argument - const auto& children = node->Children(); + const auto& children = *node->Children(); const auto& header = children[0]; const auto& args_partition = children[1]; - const auto& subpartitions = args_partition.Children(); + const auto& subpartitions = *args_partition.Children(); const auto* trailer = children.size() > 2 ? &children[2] : nullptr; const bool one_per_line = args_partition.Value().PartitionPolicy() == @@ -860,8 +954,8 @@ void ReshapeFittingSubpartitions(const BasicFormatStyle& style, uwline.SetPartitionPolicy(PartitionPolicyEnum::kFitOnLineElseExpand); // Create new grouping node - temporary_tree.Children().emplace_back(uwline); - auto* group = &temporary_tree.Children().back(); + temporary_tree.Children()->emplace_back(uwline); + auto* group = &temporary_tree.Children()->back(); // Iterate over partitions in group for (const auto& partition : itr.Children()) { @@ -869,20 +963,20 @@ void ReshapeFittingSubpartitions(const BasicFormatStyle& style, const auto* node = partition.Value().Node(); // Append child (warning contains original indentation) - group->Children().push_back(*node); + group->Children()->push_back(*node); } } // Update grouped childrens indentation in case of expanding grouping // partitions - for (auto& group : temporary_tree.Children()) { - for (auto& subpart : group.Children()) { + for (auto& group : *temporary_tree.Children()) { + for (auto& subpart : *group.Children()) { AdjustIndentationAbsolute(&subpart, group.Value().IndentationSpaces()); } } // Remove moved nodes - node->Children().clear(); + node->Children()->clear(); // Move back from temporary tree AdoptSubtreesFrom(*node, &temporary_tree); diff --git a/common/formatting/token_partition_tree.h b/common/formatting/token_partition_tree.h index e45531a52d..2006d12830 100644 --- a/common/formatting/token_partition_tree.h +++ b/common/formatting/token_partition_tree.h @@ -25,19 +25,593 @@ #include "common/formatting/unwrapped_line.h" #include "common/strings/position.h" // for ByteOffsetSet #include "common/util/container_iterator_range.h" +#include "common/util/container_proxy.h" +#include "common/util/variant.h" #include "common/util/vector_tree.h" namespace verible { +class TokenPartitionNode; +class TokenPartitionBranchNode; + +class UnwrappedLineNode; +class TokenPartitionChoiceNode; + +class TokenPartitionTree; + +// Base node classes with common functionality +// =========================================== + +// Base class for all TokenPartition nodes. +class TokenPartitionNode { + public: + // Constructors & assignment operators + + TokenPartitionNode() = default; + TokenPartitionNode(const TokenPartitionNode&) = default; + TokenPartitionNode(TokenPartitionNode&&) = default; + + TokenPartitionNode& operator=(const TokenPartitionNode& rhs) { + if (this == &rhs) { + return *this; + } + + // Intentionally keeping current parent + value_ = rhs.value_; + return *this; + } + + TokenPartitionNode& operator=(TokenPartitionNode&& rhs) noexcept { + // Intentionally keeping current parent + value_ = rhs.value_; + return *this; + } + + // + + TokenPartitionTree* Parent() { return parent_; } + const TokenPartitionTree* Parent() const { return parent_; } + + FormatTokenRange Tokens() const { return value_.TokensRange(); } + + protected: + void SetParent(TokenPartitionTree* parent) { parent_ = parent; } + + static void SetParent(TokenPartitionNode& node, TokenPartitionTree* parent) { + node.parent_ = parent; + } + + // Value-related interface + // ----------------------- + + explicit TokenPartitionNode(const UnwrappedLine& uwline) : value_(uwline) {} + + UnwrappedLine& Value() { return value_; } + const UnwrappedLine& Value() const { return value_; } + + private: + TokenPartitionTree* parent_ = nullptr; + // TODO(mglb): this is here just to provide Tokens(), which is a common + // thing in all nodes. Break UnwrappedLine into separate node members and + // move node-specific ones to appropriate subnodes. + UnwrappedLine value_; +}; + +// Base class for TokenPartition nodes providing child nodes list. +class TokenPartitionBranchNode : public TokenPartitionNode { + public: + using subnodes_type = std::vector; + + // Constructors & assignment operators + + TokenPartitionBranchNode() = default; + TokenPartitionBranchNode(const TokenPartitionBranchNode& other) = default; + TokenPartitionBranchNode(TokenPartitionBranchNode&& other) = default; + + TokenPartitionBranchNode& operator=(const TokenPartitionBranchNode& other) = + default; + + TokenPartitionBranchNode& operator=(TokenPartitionBranchNode&& other) = + default; + + // + + class ChildrenList : private ContainerProxyBase { + using Base = ContainerProxyBase; + friend Base; + + public: + using typename Base::container_type; + + // Import (via `using`) ContainerProxy members supported by std::vector. + USING_CONTAINER_PROXY_STD_VECTOR_MEMBERS(Base) + + // Move-cast to wrapped container's type. Moves out the container. + explicit operator container_type() && { return std::move(container_); } + + protected: + // ContainerProxy interface + // ------------------------ + + container_type& underlying_container() { return container_; } + const container_type& underlying_container() const { return container_; } + + void ElementsInserted(iterator first, iterator last) { + LinkChildrenToParent(iterator_range(first, last)); + } + + // void ElementsBeingRemoved(iterator first, iterator last) {} + + // void ElementsBeingReplaced() {} + + void ElementsWereReplaced() { LinkChildrenToParent(container_); } + + private: + // Sets parent pointer of nodes from `children` range to address of `node_`. + template + void LinkChildrenToParent(Range&& children); + + // Allow construction, assignment and direct access to `container_` inside + // TokenPartitionBranchNode. + friend TokenPartitionBranchNode; + + ChildrenList() = default; + + ChildrenList(const ChildrenList& other) : container_(other.container_) { + LinkChildrenToParent(container_); + } + + ChildrenList(ChildrenList&& other) noexcept + : container_(std::move(other.container_)) { + // Note: `other` is not notified about the change because it ends up in + // undefined state as a result of the move. + LinkChildrenToParent(container_); + } + + ChildrenList& operator=(const ChildrenList& other) { + container_ = other.container_; + LinkChildrenToParent(container_); + return *this; + } + + ChildrenList& operator=(ChildrenList&& other) noexcept { + // Note: `other` is not notified about the change because it ends up in + // undefined state as a result of the move. + container_ = std::move(other.container_); + LinkChildrenToParent(container_); + return *this; + } + + // Actual data container where child nodes are stored. + subnodes_type container_; + }; + + // + + ChildrenList& Children() { return children_; }; + const ChildrenList& Children() const { return children_; }; + + // + + protected: + explicit TokenPartitionBranchNode(const UnwrappedLine& uwline) + : TokenPartitionNode(uwline) {} + + // + + private: + static constexpr size_t ChildrenMemberOffset() { + // TODO(mglb): Explain this (copy-paste related comment from map_tree.h) +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + return offsetof(TokenPartitionBranchNode, children_); +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif + } + + ChildrenList children_; +}; + +// Concrete node classes +// ===================== + +class UnwrappedLineNode : public TokenPartitionBranchNode { + public: + UnwrappedLineNode() = default; + UnwrappedLineNode(const UnwrappedLineNode&) = default; + UnwrappedLineNode(UnwrappedLineNode&&) = default; + + UnwrappedLineNode& operator=(const UnwrappedLineNode&) = default; + UnwrappedLineNode& operator=(UnwrappedLineNode&&) = default; + + explicit UnwrappedLineNode(const UnwrappedLine& uwline) + : TokenPartitionBranchNode(uwline) {} + + using TokenPartitionNode::Value; +}; + +class TokenPartitionChoiceNode : public TokenPartitionNode { + using subnodes_type = std::vector; + + public: + TokenPartitionChoiceNode() = default; + TokenPartitionChoiceNode(const TokenPartitionChoiceNode&) = default; + TokenPartitionChoiceNode(TokenPartitionChoiceNode&&) = default; + + TokenPartitionChoiceNode& operator=(const TokenPartitionChoiceNode&) = + default; + TokenPartitionChoiceNode& operator=(TokenPartitionChoiceNode&&) = default; + + explicit TokenPartitionChoiceNode(const UnwrappedLine& uwline) + : TokenPartitionNode(uwline) {} + + class ChoicesList : ContainerProxyBase { + using Base = ContainerProxyBase; + friend Base; + + public: + using typename Base::container_type; + + // Import (via `using`) ContainerProxy members supported by std::vector. + USING_CONTAINER_PROXY_STD_VECTOR_MEMBERS(Base) + + // Move-cast to wrapped container's type. Moves out the container. + explicit operator subnodes_type() && { return std::move(container_); } + + protected: + // ContainerProxy interface + + container_type& underlying_container() { return container_; } + const container_type& underlying_container() const { return container_; } + + void ElementsInserted(iterator first, iterator last) { + VerifyAndProcessNewChoices(iterator_range(first, last)); + } + + // void ElementsBeingRemoved(iterator first, iterator last) {} + + // void ElementsBeingReplaced() {} + + void ElementsWereReplaced() { VerifyAndProcessNewChoices(container_); } + + private: + template + void VerifyAndProcessNewChoices(Range&& choice_subtrees); + + // Allow assignment inside TokenPartitionChoiceNode. + friend TokenPartitionChoiceNode; + + ChoicesList() = default; + + ChoicesList(const ChoicesList& other) : container_(other.container_) { + VerifyAndProcessNewChoices(container_); + } + + ChoicesList(ChoicesList&& other) noexcept + : container_(std::move(other.container_)) { + // Note: `other` is not notified about the change because it ends up in + // undefined state as a result of the move. + VerifyAndProcessNewChoices(container_); + } + + ChoicesList& operator=(const ChoicesList& other) { + container_ = other.container_; + VerifyAndProcessNewChoices(container_); + return *this; + } + + ChoicesList& operator=(ChoicesList&& other) noexcept { + // Note: `other` is not notified about the change because it ends up in + // undefined state as a result of the move. + container_ = std::move(other.container_); + VerifyAndProcessNewChoices(container_); + return *this; + } + + // Actual data container where choice subtrees are stored. + subnodes_type container_; + }; + + ChoicesList& Choices() { return choices_; } + const ChoicesList& Choices() const { return choices_; } + + using TokenPartitionNode::Value; + + private: + static constexpr size_t ChoicesMemberOffset() { +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif + return offsetof(TokenPartitionChoiceNode, choices_); +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif + } + + ChoicesList choices_; +}; + +// TokenPartitionTreePrinter +// ========================= + +// Custom printer, alternative to the default stream operator<<. +// Modeled after VectorTree<>::PrintTree, but suppresses printing of the +// tokens for non-leaf nodes because a node's token range always spans +// that of all of its children. +// Usage: stream << TokenPartitionTreePrinter(tree) << std::endl; +struct TokenPartitionTreePrinter { + explicit TokenPartitionTreePrinter( + const TokenPartitionTree& n, bool verbose = false, + UnwrappedLine::OriginPrinterFunction origin_printer = + UnwrappedLine::DefaultOriginPrinter) + : node(n), verbose(verbose), origin_printer(std::move(origin_printer)) {} + + std::ostream& PrintTree(std::ostream& stream, int indent = 0) const; + + // The (sub)tree to display. + const TokenPartitionTree& node; + // If true, display inter-token information. + bool verbose; + + UnwrappedLine::OriginPrinterFunction origin_printer; +}; + +std::ostream& operator<<(std::ostream& stream, + const TokenPartitionTreePrinter& printer); + +// TokenPartitionTree +// ================== + +using TokenPartitionVariant = + verible::Variant; + // Opaque type alias for a hierarchically partitioned format token stream. // Objects of this type maintain the following invariants: // 1) The format token range spanned by any tree node (UnwrappedLine) is // equal to that of its children. // 2) Adjacent siblings begin/end iterators are equal (continuity). // -// TODO(fangism): Promote this to a class that privately inherits the base. -// Methods on this class will preserve invariants. -using TokenPartitionTree = VectorTree; +class TokenPartitionTree : public TokenPartitionVariant { + public: + // Type alias for TreeNode interface + using subnodes_type = TokenPartitionBranchNode::subnodes_type; + + // TODO(mglb): replace using class hierarchy + enum class Kind : int8_t { + kPartition, + kChoice, + }; + + friend std::ostream& operator<<(std::ostream& s, Kind kind) { + switch (kind) { + case Kind::kPartition: + return s << "partition"; + case Kind::kChoice: + return s << "choice"; + default: + return s << ""; + } + } + + TokenPartitionTree() = default; + + explicit TokenPartitionTree(Kind kind) + : TokenPartitionVariant( + (kind == Kind::kPartition) + ? TokenPartitionVariant(in_place_type) + : TokenPartitionVariant( + in_place_type)) {} + + TokenPartitionTree(const TokenPartitionTree& other) = default; + + TokenPartitionTree(TokenPartitionTree&& other) = default; + + // This constructor can be used to recursively build trees. + // e.g. + // // looks like function-call, but just invokes constructor: + // typedef TokenPartitionTree FooNode; + // auto foo_tree = FooNode({value-initializer}, + // FooNode({value-initializer}, /* children nodes... */ ), + // FooNode({value-initializer}, /* children nodes... */ ) + // ); + template + explicit TokenPartitionTree(const UnwrappedLine& v, Args&&... args) + : TokenPartitionVariant(in_place_type, v) { + auto& node = get(*this); + node.Children().reserve(sizeof...(args)); + (node.Children().emplace_back(std::forward(args)), ...); + } + + template + explicit TokenPartitionTree(UnwrappedLine&& v, Args&&... args) + : TokenPartitionVariant(in_place_type, v) { + auto& node = get(*this); + node.Children().reserve(sizeof...(args)); + (node.Children().emplace_back(std::forward(args)), ...); + } + + ~TokenPartitionTree() {} + + // Swaps values and subtrees of two nodes. + // This operation is safe for unrelated trees (no common ancestor). + // This operation is safe when the two nodes share a common ancestor, + // excluding the case where one node is a direct ancestor of the other. + // TODO(fangism): Add a proper check for this property, and test. + using TokenPartitionVariant::swap; + + TokenPartitionTree& operator=(const TokenPartitionTree& source) { + TokenPartitionVariant::operator=(source); + return *this; + } + + TokenPartitionTree& operator=(TokenPartitionTree&& source) noexcept { + TokenPartitionVariant::operator=(std::move(source)); + return *this; + } + + // TreeNode interface: + + UnwrappedLine& Value() { + return Visit([](auto& node) -> UnwrappedLine& { return node.Value(); }, + *this); + } + const UnwrappedLine& Value() const { + return Visit( + [](const auto& node) -> const UnwrappedLine& { return node.Value(); }, + *this); + } + + TokenPartitionTree* Parent() { + return Visit([](TokenPartitionNode& node) { return node.Parent(); }, *this); + } + const TokenPartitionTree* Parent() const { + return Visit([](const TokenPartitionNode& node) { return node.Parent(); }, + *this); + } + + TokenPartitionBranchNode::ChildrenList* Children() { + using ChildrenList = TokenPartitionBranchNode::ChildrenList; + return Visit( + Overload{ + [](TokenPartitionBranchNode& node) -> ChildrenList* { + return &node.Children(); + }, + [](TokenPartitionNode& node) -> ChildrenList* { return nullptr; }, + }, + *this); + } + const TokenPartitionBranchNode::ChildrenList* Children() const { + using ChildrenList = TokenPartitionBranchNode::ChildrenList; + return Visit( + Overload{ + [](const TokenPartitionBranchNode& node) -> const ChildrenList* { + return &node.Children(); + }, + [](const TokenPartitionNode& node) -> const ChildrenList* { + return nullptr; + }, + }, + *this); + } + + // Choice node interface + + auto& Choices() { + using ChoicesList = TokenPartitionChoiceNode::ChoicesList; + return *Visit(Overload{ + [](TokenPartitionChoiceNode& node) -> ChoicesList* { + return &node.Choices(); + }, + [](TokenPartitionNode& node) -> ChoicesList* { + LOG(FATAL) << "Unexpected node type"; + return nullptr; + }, + }, + *this); + } + + const auto& Choices() const { + using ChoicesList = TokenPartitionChoiceNode::ChoicesList; + return *Visit( + Overload{ + [](const TokenPartitionChoiceNode& node) -> const ChoicesList* { + return &node.Choices(); + }, + [](const TokenPartitionNode& node) -> const ChoicesList* { + LOG(FATAL) << "Unexpected node type"; + return nullptr; + }, + }, + *this); + } + + // Node type checks: + + Kind kind() const { + return Visit( + Overload{ + [](const TokenPartitionChoiceNode&) { return Kind::kChoice; }, + [](const UnwrappedLineNode&) { return Kind::kPartition; }, + }, + *this); + } + + bool IsChoice() const { return kind() == Kind::kChoice; } + + bool IsPartition() const { return kind() == Kind::kPartition; } + + bool IsLeafPartition() const { + return kind() == Kind::kPartition && Children()->empty(); + } + + bool IsAlreadyFormatted() const { + return kind() == Kind::kPartition && + Value().PartitionPolicy() == PartitionPolicyEnum::kAlreadyFormatted; + } +}; + +// Node implementations +// ==================== + +template +void TokenPartitionBranchNode::ChildrenList::LinkChildrenToParent( + Range&& children) { + // FIXME(mglb): Fragile magic - document! + + static constexpr size_t offset_in_node = + TokenPartitionBranchNode::ChildrenMemberOffset(); + + std::byte* const this_ptr = reinterpret_cast(this); + std::byte* const node_ptr = this_ptr - offset_in_node; + + TokenPartitionBranchNode* const node = + reinterpret_cast(node_ptr); + + TokenPartitionTree* variant = std::launder(static_cast( + TokenPartitionTree::GetFromStoredObject(node))); + + for (auto& child : children) { + Visit( + [variant](TokenPartitionNode& node) { + TokenPartitionNode::SetParent(node, variant); + }, + child); + } +} + +template +void TokenPartitionChoiceNode::ChoicesList::VerifyAndProcessNewChoices( + Range&& choice_subtrees) { + // FIXME(mglb): Fragile magic - document! + + static constexpr size_t offset_in_node = + TokenPartitionChoiceNode::ChoicesMemberOffset(); + + std::byte* const this_ptr = reinterpret_cast(this); + std::byte* const node_ptr = this_ptr - offset_in_node; + + TokenPartitionChoiceNode* const node = + std::launder(reinterpret_cast(node_ptr)); + + for (auto& subtree : choice_subtrees) { + Visit( + [node](TokenPartitionNode& subnode) { + CHECK(subnode.Tokens() == node->Tokens()) + << "Each choice subtree must cover the same tokens as the Choice " + "node."; + TokenPartitionNode::SetParent(subnode, nullptr); + }, + subtree); + } +} + +// + +// using TokenPartitionTree = VectorTree; using TokenPartitionIterator = TokenPartitionTree::subnodes_type::iterator; using TokenPartitionRange = container_iterator_range; @@ -69,31 +643,6 @@ std::vector FindLargestPartitions( std::vector> FlushLeftSpacingDifferences( const TokenPartitionRange& partitions); -// Custom printer, alternative to the default stream operator<<. -// Modeled after VectorTree<>::PrintTree, but suppresses printing of the -// tokens for non-leaf nodes because a node's token range always spans -// that of all of its children. -// Usage: stream << TokenPartitionTreePrinter(tree) << std::endl; -struct TokenPartitionTreePrinter { - explicit TokenPartitionTreePrinter( - const TokenPartitionTree& n, bool verbose = false, - UnwrappedLine::OriginPrinterFunction origin_printer = - UnwrappedLine::DefaultOriginPrinter) - : node(n), verbose(verbose), origin_printer(std::move(origin_printer)) {} - - std::ostream& PrintTree(std::ostream& stream, int indent = 0) const; - - // The (sub)tree to display. - const TokenPartitionTree& node; - // If true, display inter-token information. - bool verbose; - - UnwrappedLine::OriginPrinterFunction origin_printer; -}; - -std::ostream& operator<<(std::ostream& stream, - const TokenPartitionTreePrinter& printer); - // Returns true if any substring range spanned by the token partition 'range' is // disabled by 'disabled_byte_ranges'. // 'full_text' is the original string_view that spans the text from which diff --git a/common/formatting/token_partition_tree_test.cc b/common/formatting/token_partition_tree_test.cc index 822a10dacc..990d70e1c5 100644 --- a/common/formatting/token_partition_tree_test.cc +++ b/common/formatting/token_partition_tree_test.cc @@ -56,6 +56,65 @@ class TokenPartitionTreeTestFixture : public ::testing::Test, std::vector ftokens_; }; +class ChoicesNodeTest : public TokenPartitionTreeTestFixture {}; + +TEST_F(ChoicesNodeTest, Misc) { + using TPT = TokenPartitionTreeBuilder; + + auto tree_with_choice = + TPT(PartitionPolicyEnum::kStack, + { + TPT(2, {0, 1}, PartitionPolicyEnum::kFitOnLineElseExpand), + TPT(2, {1, 5}, PartitionPolicyEnum::kUninitialized), + TPT(2, {5, 6}, PartitionPolicyEnum::kFitOnLineElseExpand), + }) + .build(pre_format_tokens_); + + auto& choice_node = tree_with_choice.Children()->at(1); + { + auto choice_data = choice_node.Value(); + choice_node = TokenPartitionTree(TokenPartitionTree::Kind::kChoice); + choice_node.Value() = choice_data; + } + + EXPECT_EQ(choice_node.Children(), nullptr); + EXPECT_TRUE(choice_node.Choices().empty()); + EXPECT_EQ(choice_node.kind(), TokenPartitionTree::Kind::kChoice); + + choice_node.Choices().push_back(TPT(PartitionPolicyEnum::kFitOnLineElseExpand, + { + TPT({1, 3}), + TPT({3, 5}), + }) + .build(pre_format_tokens_)); + choice_node.Choices().push_back(TPT(PartitionPolicyEnum::kAlwaysExpand, + { + TPT({1, 2}), + TPT({2, 3}), + TPT({3, 4}), + TPT({4, 5}), + }) + .build(pre_format_tokens_)); + + EXPECT_DEATH( + choice_node.Choices().push_back(TPT(PartitionPolicyEnum::kAlwaysExpand, + { + // Range != choice node range. + TPT({1, 2}), + }) + .build(pre_format_tokens_)), + "must cover the same tokens"); + + EXPECT_EQ(choice_node.Children(), nullptr); + EXPECT_EQ(choice_node.Choices().size(), 2); + EXPECT_EQ(choice_node.Choices()[0].Parent(), nullptr); + EXPECT_EQ(choice_node.Choices()[0].kind(), + TokenPartitionTree::Kind::kPartition); + EXPECT_EQ(choice_node.Choices()[1].Parent(), nullptr); + EXPECT_EQ(choice_node.Choices()[1].kind(), + TokenPartitionTree::Kind::kPartition); +} + class VerifyFullTreeFormatTokenRangesTest : public TokenPartitionTreeTestFixture {}; @@ -180,8 +239,8 @@ TEST_F(FlushLeftSpacingDifferencesTest, EmptyPartitions) { const auto begin = preformat_tokens.begin(); UnwrappedLine all(0, begin); TokenPartitionTree tree{all}; // no children - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); using V = std::vector; const std::vector diffs(FlushLeftSpacingDifferences(range)); EXPECT_THAT(diffs, ElementsAre()); @@ -196,8 +255,8 @@ TEST_F(FlushLeftSpacingDifferencesTest, OnePartitionsOneToken) { partition0.SpanUpToToken(begin + 1); using T = TokenPartitionTree; T tree{partition0, T{partition0}}; // one partition, one token - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); using V = std::vector; const std::vector diffs(FlushLeftSpacingDifferences(range)); EXPECT_THAT(diffs, ElementsAre(V())); @@ -212,8 +271,8 @@ TEST_F(FlushLeftSpacingDifferencesTest, OnePartitionsTwoTokens) { partition0.SpanUpToToken(begin + 2); using T = TokenPartitionTree; T tree{partition0, T{partition0}}; // one partition, two tokens - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); using V = std::vector; const std::vector diffs(FlushLeftSpacingDifferences(range)); EXPECT_THAT(diffs, ElementsAre(V({0}))); @@ -230,8 +289,8 @@ TEST_F(FlushLeftSpacingDifferencesTest, TwoPartitionsOneTokenEach) { partition1.SpanUpToToken(begin + 2); using T = TokenPartitionTree; T tree{partition0, T{partition0}, T{partition1}}; // two partitions - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); using V = std::vector; const std::vector diffs(FlushLeftSpacingDifferences(range)); EXPECT_THAT(diffs, ElementsAre(V(), V())); @@ -248,8 +307,8 @@ TEST_F(FlushLeftSpacingDifferencesTest, TwoPartitionsTwoTokensEach) { partition1.SpanUpToToken(begin + 4); using T = TokenPartitionTree; T tree{partition0, T{partition0}, T{partition1}}; // two partitions - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); using V = std::vector; const std::vector diffs(FlushLeftSpacingDifferences(range)); EXPECT_THAT(diffs, ElementsAre(V({0}), V({0}))); @@ -375,19 +434,19 @@ TEST_F(AdjustIndentationTest, Relative) { tree_type{sub}, // same range, but indented more }; EXPECT_EQ(tree.Value().IndentationSpaces(), 5); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 7); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 7); AdjustIndentationRelative(&tree, 1); EXPECT_EQ(tree.Value().IndentationSpaces(), 6); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 8); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 8); AdjustIndentationRelative(&tree, -3); EXPECT_EQ(tree.Value().IndentationSpaces(), 3); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 5); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 5); AdjustIndentationRelative(&tree, -4); EXPECT_EQ(tree.Value().IndentationSpaces(), 0); // clamped at 0 - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 1); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 1); } TEST_F(AdjustIndentationTest, Absolute) { @@ -405,19 +464,19 @@ TEST_F(AdjustIndentationTest, Absolute) { tree_type{sub}, // same range, but indented more }; EXPECT_EQ(tree.Value().IndentationSpaces(), 5); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 7); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 7); AdjustIndentationAbsolute(&tree, 5); // no change EXPECT_EQ(tree.Value().IndentationSpaces(), 5); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 7); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 7); AdjustIndentationAbsolute(&tree, 8); EXPECT_EQ(tree.Value().IndentationSpaces(), 8); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 10); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 10); AdjustIndentationAbsolute(&tree, 0); EXPECT_EQ(tree.Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children().front().Value().IndentationSpaces(), 2); + EXPECT_EQ(tree.Children()->front().Value().IndentationSpaces(), 2); } static bool PropertiesEqual(const UnwrappedLine& left, @@ -466,7 +525,7 @@ TEST_F(GroupLeafWithPreviousLeafTest, OneChild) { ASSERT_FALSE(is_leaf(tree)); const auto saved_tree(tree); // deep copy - auto* group = GroupLeafWithPreviousLeaf(&tree.Children().front()); + auto* group = GroupLeafWithPreviousLeaf(&tree.Children()->front()); EXPECT_EQ(group, nullptr); // Expect no change. @@ -513,8 +572,8 @@ TEST_F(GroupLeafWithPreviousLeafTest, TwoChild) { }, }; - auto* group = GroupLeafWithPreviousLeaf(&tree.Children().back()); - EXPECT_EQ(group, &tree.Children().back()); + auto* group = GroupLeafWithPreviousLeaf(&tree.Children()->back()); + EXPECT_EQ(group, &tree.Children()->back()); const auto diff = DeepEqual(tree, expected_tree, PropertiesEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -546,8 +605,9 @@ TEST_F(GroupLeafWithPreviousLeafTest, TwoGenerations) { tree_type tree{ all, tree_type{ - part1, tree_type{part1a}, // unchanged - tree_type{part1b}, // target partition + part1, // + tree_type{part1a}, // unchanged + tree_type{part1b}, // target partition }, tree_type{part2}, // source partition }; @@ -558,14 +618,15 @@ TEST_F(GroupLeafWithPreviousLeafTest, TwoGenerations) { all, tree_type{part1a}, tree_type{ - group_part, tree_type{part1b}, // target partition - tree_type{part2}, // source partition + group_part, // + tree_type{part1b}, // target partition + tree_type{part2}, // source partition }, }, }; - auto* group = GroupLeafWithPreviousLeaf(&tree.Children().back()); - EXPECT_EQ(group, &tree.Children().back().Children().back()); + auto* group = GroupLeafWithPreviousLeaf(&tree.Children()->back()); + EXPECT_EQ(group, &tree.Children()->back().Children()->back()); const auto diff = DeepEqual(tree, expected_tree, PropertiesEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -630,8 +691,8 @@ TEST_F(GroupLeafWithPreviousLeafTest, Cousins) { }; auto* group = - GroupLeafWithPreviousLeaf(&tree.Children().back().Children().front()); - EXPECT_EQ(group, &tree.Children().front().Children().back()); + GroupLeafWithPreviousLeaf(&tree.Children()->back().Children()->front()); + EXPECT_EQ(group, &tree.Children()->front().Children()->back()); const auto diff = DeepEqual(tree, expected_tree, PropertiesEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -675,8 +736,8 @@ TEST_F(GroupLeafWithPreviousLeafTest, ExtendCommonAncestor) { }, }; - auto* group = GroupLeafWithPreviousLeaf(&tree.Children().back()); - EXPECT_EQ(group, &tree.Children().back()); + auto* group = GroupLeafWithPreviousLeaf(&tree.Children()->back()); + EXPECT_EQ(group, &tree.Children()->back()); const auto diff = DeepEqual(tree, expected_tree, PropertiesEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -728,7 +789,7 @@ TEST_F(MergeLeafIntoPreviousLeafTest, OneChild) { ASSERT_FALSE(is_leaf(tree)); const auto saved_tree(tree); // deep copy - auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children().front()); + auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children()->front()); EXPECT_EQ(parent, nullptr); // Expect no change. @@ -762,7 +823,7 @@ TEST_F(MergeLeafIntoPreviousLeafTest, TwoChild) { tree_type{all}, }; - auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children().back()); + auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children()->back()); EXPECT_EQ(parent, &tree); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); @@ -809,7 +870,7 @@ TEST_F(MergeLeafIntoPreviousLeafTest, TwoGenerations) { }, }; - auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children().back()); + auto* parent = MergeLeafIntoPreviousLeaf(&tree.Children()->back()); EXPECT_EQ(parent, &tree); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); @@ -871,8 +932,8 @@ TEST_F(MergeLeafIntoPreviousLeafTest, Cousins) { }; auto* parent = - MergeLeafIntoPreviousLeaf(&tree.Children().back().Children().front()); - EXPECT_EQ(parent, &tree.Children().back()); + MergeLeafIntoPreviousLeaf(&tree.Children()->back().Children()->front()); + EXPECT_EQ(parent, &tree.Children()->back()); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -919,7 +980,7 @@ TEST_F(MergeLeafIntoNextLeafTest, OneChild) { ASSERT_FALSE(is_leaf(tree)); const auto saved_tree(tree); // deep copy - auto* parent = MergeLeafIntoNextLeaf(&tree.Children().front()); + auto* parent = MergeLeafIntoNextLeaf(&tree.Children()->front()); EXPECT_EQ(parent, nullptr); // Expect no change. @@ -953,7 +1014,7 @@ TEST_F(MergeLeafIntoNextLeafTest, TwoChild) { tree_type{all}, }; - auto* parent = MergeLeafIntoNextLeaf(&tree.Children().front()); + auto* parent = MergeLeafIntoNextLeaf(&tree.Children()->front()); EXPECT_EQ(parent, &tree); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); @@ -1002,8 +1063,8 @@ TEST_F(MergeLeafIntoNextLeafTest, TwoGenerations) { }; auto* parent = - MergeLeafIntoNextLeaf(&tree.Children().front().Children().back()); - EXPECT_EQ(parent, &tree.Children().front()); + MergeLeafIntoNextLeaf(&tree.Children()->front().Children()->back()); + EXPECT_EQ(parent, &tree.Children()->front()); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -1065,8 +1126,8 @@ TEST_F(MergeLeafIntoNextLeafTest, Cousins) { }; auto* parent = - MergeLeafIntoNextLeaf(&tree.Children().front().Children().back()); - EXPECT_EQ(parent, &tree.Children().front()); + MergeLeafIntoNextLeaf(&tree.Children()->front().Children()->back()); + EXPECT_EQ(parent, &tree.Children()->front()); const auto diff = DeepEqual(tree, expected_tree, TokenRangeEqual); EXPECT_TRUE(diff.left == nullptr) << "First differing node at:\n" @@ -1203,14 +1264,14 @@ TEST_F(AnyPartitionSubRangeIsDisabledTest, Various) { }; { - const TokenPartitionRange empty(tree.Children().begin(), - tree.Children().begin()); + const TokenPartitionRange empty(tree.Children()->begin(), + tree.Children()->begin()); EXPECT_FALSE(AnyPartitionSubRangeIsDisabled(empty, joined_token_text_, ByteOffsetSet{})); } { - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); EXPECT_FALSE(AnyPartitionSubRangeIsDisabled(range, joined_token_text_, ByteOffsetSet{})); EXPECT_TRUE(AnyPartitionSubRangeIsDisabled(range, joined_token_text_, @@ -1285,8 +1346,8 @@ class GetSubpartitionsNoNewlinesTest }; TEST_F(GetSubpartitionsNoNewlinesTest, NoNewlines) { - const TokenPartitionRange range(partition_.Children().begin(), - partition_.Children().end()); + const TokenPartitionRange range(partition_.Children()->begin(), + partition_.Children()->end()); const auto partition_ranges = GetSubpartitionsBetweenBlankLines(range); EXPECT_THAT(partition_ranges, ElementsAre(TokenPartitionRange(range.begin(), range.end()))); @@ -1301,8 +1362,8 @@ class GetSubpartitionsNoBlanksTest }; TEST_F(GetSubpartitionsNoBlanksTest, NoBlanks) { - const TokenPartitionRange range(partition_.Children().begin(), - partition_.Children().end()); + const TokenPartitionRange range(partition_.Children()->begin(), + partition_.Children()->end()); const auto partition_ranges = GetSubpartitionsBetweenBlankLines(range); EXPECT_THAT(partition_ranges, ElementsAre(TokenPartitionRange(range.begin(), range.end()))); @@ -1317,8 +1378,8 @@ class GetSubpartitionsWithBlanksTest }; TEST_F(GetSubpartitionsWithBlanksTest, WithBlanks) { - const TokenPartitionRange range(partition_.Children().begin(), - partition_.Children().end()); + const TokenPartitionRange range(partition_.Children()->begin(), + partition_.Children()->end()); const auto partition_ranges = GetSubpartitionsBetweenBlankLines(range); EXPECT_THAT( partition_ranges, @@ -1352,8 +1413,8 @@ TEST_F(IndentButPreserveOtherSpacingTest, Various) { tree_type{partition2}, }; - const TokenPartitionRange range(tree.Children().begin(), - tree.Children().end()); + const TokenPartitionRange range(tree.Children()->begin(), + tree.Children()->end()); IndentButPreserveOtherSpacing(range, joined_token_text_, &pre_format_tokens_); // The first tokens on each partition need not be preserved, but all // subsequent tokens should be preserved. @@ -1396,7 +1457,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, NoArguments) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentInSubpartition) { @@ -1438,7 +1499,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentInSubpartition) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentInSubpartitionAndTrailer) { @@ -1484,7 +1545,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentInSubpartitionAndTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentFlat) { @@ -1521,7 +1582,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentFlat) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentFlatAndTrailer) { @@ -1564,7 +1625,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OneArgumentFlatAndTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, TwoArguments) { @@ -1614,7 +1675,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, TwoArguments) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } TEST_F(ReshapeFittingSubpartitionsTest, TwoArgumentsAndTrailer) { @@ -1669,7 +1730,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, TwoArgumentsAndTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } // All fits in one partition @@ -1732,7 +1793,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OnePartition) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } // All fits in one partition @@ -1796,7 +1857,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, OnePartitionWithTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); } // Wrap into two partitions @@ -1868,9 +1929,9 @@ TEST_F(ReshapeFittingSubpartitionsTest, TwoPartitions) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); EXPECT_EQ( - tree.Children()[1].Value().IndentationSpaces(), + tree.Children()->at(1).Value().IndentationSpaces(), header.TokensRange()[0].Length()); // indenation equal first token length } @@ -1944,9 +2005,9 @@ TEST_F(ReshapeFittingSubpartitionsTest, TwoPartitionsWithTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); EXPECT_EQ( - tree.Children()[1].Value().IndentationSpaces(), + tree.Children()->at(1).Value().IndentationSpaces(), header.TokensRange()[0].Length()); // indenation equal first token length } @@ -1992,7 +2053,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, NoneOneFits) { }, }; - ApplyPreOrder(tree.Children()[1], [&style](TokenPartitionTree& node) { + ApplyPreOrder(tree.Children()->at(1), [&style](TokenPartitionTree& node) { auto& uwline = node.Value(); uwline.SetIndentationSpaces(style.wrap_spaces); }); @@ -2013,11 +2074,15 @@ TEST_F(ReshapeFittingSubpartitionsTest, NoneOneFits) { EXPECT_EQ(diff.left, nullptr) << "Expected:\n" << tree_expected << "\nGot:" << tree << "\n"; - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[4].Value().IndentationSpaces(), style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(4).Value().IndentationSpaces(), + style.wrap_spaces); } // None fits @@ -2079,12 +2144,16 @@ TEST_F(ReshapeFittingSubpartitionsTest, NoneOneFitsWithTrailer) { EXPECT_EQ(diff.left, nullptr) << "Expected:\n" << tree_expected << "\nGot:" << tree << "\n"; - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[4].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[5].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(4).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(5).Value().IndentationSpaces(), 0); } // All fits in one partition @@ -2128,12 +2197,12 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationOnePartition) { }, }; - ApplyPreOrder(tree.Children()[1], [&style](TokenPartitionTree& node) { + ApplyPreOrder(tree.Children()->at(1), [&style](TokenPartitionTree& node) { auto& uwline = node.Value(); uwline.SetIndentationSpaces(3 + style.indentation_spaces); }); - tree.Children()[0].Value().SetIndentationSpaces(3); + tree.Children()->at(0).Value().SetIndentationSpaces(3); tree.Value().SetIndentationSpaces(3); const tree_type tree_expected{ @@ -2157,7 +2226,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationOnePartition) { << tree_expected << "\nGot:" << tree << "\n"; // keep orig indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), saved_tree.Value().IndentationSpaces()); } @@ -2224,7 +2293,7 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationOnePartitionWithTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // keep orig indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), saved_tree.Value().IndentationSpaces()); } @@ -2270,12 +2339,12 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationTwoPartitions) { verible::BasicFormatStyle style; // default style.column_limit = 14 + 7; - ApplyPreOrder(tree.Children()[1], [&style](TokenPartitionTree& node) { + ApplyPreOrder(tree.Children()->at(1), [&style](TokenPartitionTree& node) { auto& uwline = node.Value(); uwline.SetIndentationSpaces(7 + style.indentation_spaces); }); - tree.Children()[0].Value().SetIndentationSpaces(7); + tree.Children()->at(0).Value().SetIndentationSpaces(7); tree.Value().SetIndentationSpaces(7); UnwrappedLine group1(0, header.TokensRange().begin()); @@ -2307,21 +2376,27 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationTwoPartitions) { << tree_expected << "\nGot:" << tree << "\n"; // Check group nodes indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 7); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(saved_tree.Children()[0].Value().IndentationSpaces(), - tree.Children()[0].Children()[0].Value().IndentationSpaces()); - EXPECT_EQ(tree.Children()[0].Children()[1].Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[0].Children()[2].Value().IndentationSpaces(), 7); + EXPECT_EQ( + saved_tree.Children()->at(0).Value().IndentationSpaces(), + tree.Children()->at(0).Children()->at(0).Value().IndentationSpaces()); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(1).Value().IndentationSpaces(), 7); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(2).Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[1].Children()[0].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(tree.Children()[1].Children()[1].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(tree.Children()[1].Children()[2].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(0).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(1).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(2).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended } // Wrap into two partitions @@ -2396,21 +2471,27 @@ TEST_F(ReshapeFittingSubpartitionsTest, IndentationTwoPartitionsWithTrailer) { << tree_expected << "\nGot:" << tree << "\n"; // Check group nodes indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 7); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(saved_tree.Children()[0].Value().IndentationSpaces(), - tree.Children()[0].Children()[0].Value().IndentationSpaces()); - EXPECT_EQ(tree.Children()[0].Children()[1].Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[0].Children()[2].Value().IndentationSpaces(), 7); + EXPECT_EQ( + saved_tree.Children()->at(0).Value().IndentationSpaces(), + tree.Children()->at(0).Children()->at(0).Value().IndentationSpaces()); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(1).Value().IndentationSpaces(), 7); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(2).Value().IndentationSpaces(), 7); - EXPECT_EQ(tree.Children()[1].Children()[0].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(tree.Children()[1].Children()[1].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended - EXPECT_EQ(tree.Children()[1].Children()[2].Value().IndentationSpaces(), - header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(0).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(1).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended + EXPECT_EQ( + tree.Children()->at(1).Children()->at(2).Value().IndentationSpaces(), + header.TokensRange()[0].Length() + 7); // appended } // One subpartition per line @@ -2479,26 +2560,31 @@ TEST_F(ReshapeFittingSubpartitionsTest, OnePerLine) { // Check group nodes indentation const int header_width = header.TokensRange()[0].Length(); - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 1 * kIndent); - EXPECT_EQ(tree.Children()[0].Children()[0].Value().IndentationSpaces(), - 1 * kIndent); - EXPECT_EQ(tree.Children()[0].Children()[1].Value().IndentationSpaces(), - 1 * kIndent); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 1 * kIndent); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(1).Value().IndentationSpaces(), + 1 * kIndent); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[1].Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(1).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[2].Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(2).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[3].Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(3).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); } // One subpartition per line @@ -2577,28 +2663,34 @@ TEST_F(ReshapeFittingSubpartitionsTest, OnePerLineWithTrailer) { // Check group nodes indentation const int header_width = header.TokensRange()[0].Length(); - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 1 * kIndent); - EXPECT_EQ(tree.Children()[0].Children()[0].Value().IndentationSpaces(), - 1 * kIndent); - EXPECT_EQ(tree.Children()[0].Children()[1].Value().IndentationSpaces(), - 1 * kIndent); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 1 * kIndent); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(1).Value().IndentationSpaces(), + 1 * kIndent); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[1].Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(1).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[2].Children()[0].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(2).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[3].Children()[0].Value().IndentationSpaces(), - 1 * kIndent + header_width); - EXPECT_EQ(tree.Children()[3].Children()[1].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(3).Children()->at(0).Value().IndentationSpaces(), + 1 * kIndent + header_width); + EXPECT_EQ( + tree.Children()->at(3).Children()->at(1).Value().IndentationSpaces(), + 1 * kIndent + header_width); } TEST_F(ReshapeFittingSubpartitionsTest, AvoidExceedingColumnLimit) { @@ -2672,19 +2764,25 @@ TEST_F(ReshapeFittingSubpartitionsTest, AvoidExceedingColumnLimit) { << tree_expected << "\nGot:" << tree << "\n"; // Check group nodes indentation - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[0].Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ( + tree.Children()->at(0).Children()->at(0).Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[1].Children()[0].Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[1].Children()[1].Value().IndentationSpaces(), 1); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), 1); + EXPECT_EQ( + tree.Children()->at(1).Children()->at(0).Value().IndentationSpaces(), 1); + EXPECT_EQ( + tree.Children()->at(1).Children()->at(1).Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[2].Children()[0].Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[2].Children()[1].Value().IndentationSpaces(), 1); + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), 1); + EXPECT_EQ( + tree.Children()->at(2).Children()->at(0).Value().IndentationSpaces(), 1); + EXPECT_EQ( + tree.Children()->at(2).Children()->at(1).Value().IndentationSpaces(), 1); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[3].Children()[0].Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), 0); + EXPECT_EQ( + tree.Children()->at(3).Children()->at(0).Value().IndentationSpaces(), 0); } // Tests with real-world example @@ -2763,7 +2861,7 @@ TEST_F(ReshapeFittingSubpartitionsFunctionTest, verible::BasicFormatStyle style; style.column_limit = 20; - ApplyPreOrder(tree.Children()[1], [&style](TokenPartitionTree& node) { + ApplyPreOrder(tree.Children()->at(1), [&style](TokenPartitionTree& node) { auto& uwline = node.Value(); uwline.SetIndentationSpaces(style.wrap_spaces); }); @@ -2787,13 +2885,19 @@ TEST_F(ReshapeFittingSubpartitionsFunctionTest, << tree_expected << "\nGot:" << tree << "\n"; // Check indentations - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[4].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[5].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[6].Value().IndentationSpaces(), style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(4).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(5).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(6).Value().IndentationSpaces(), + style.wrap_spaces); } TEST_F(ReshapeFittingSubpartitionsFunctionTest, @@ -2843,7 +2947,7 @@ TEST_F(ReshapeFittingSubpartitionsFunctionTest, verible::BasicFormatStyle style; style.column_limit = 40; - ApplyPreOrder(tree.Children()[1], [&style](TokenPartitionTree& node) { + ApplyPreOrder(tree.Children()->at(1), [&style](TokenPartitionTree& node) { auto& uwline = node.Value(); uwline.SetIndentationSpaces(style.wrap_spaces); }); @@ -2885,10 +2989,13 @@ TEST_F(ReshapeFittingSubpartitionsFunctionTest, << tree_expected << "\nGot:" << tree << "\n"; // Check indentations - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), style.wrap_spaces); - EXPECT_EQ(tree.Children()[3].Value().IndentationSpaces(), style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), + style.wrap_spaces); + EXPECT_EQ(tree.Children()->at(3).Value().IndentationSpaces(), + style.wrap_spaces); } TEST_F(ReshapeFittingSubpartitionsFunctionTest, @@ -2969,10 +3076,10 @@ TEST_F(ReshapeFittingSubpartitionsFunctionTest, << tree_expected << "\nGot:" << tree << "\n"; // Check indentations (appending) - EXPECT_EQ(tree.Children()[0].Value().IndentationSpaces(), 0); - EXPECT_EQ(tree.Children()[1].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(0).Value().IndentationSpaces(), 0); + EXPECT_EQ(tree.Children()->at(1).Value().IndentationSpaces(), header.TokensRange()[0].Length()); - EXPECT_EQ(tree.Children()[2].Value().IndentationSpaces(), + EXPECT_EQ(tree.Children()->at(2).Value().IndentationSpaces(), header.TokensRange()[0].Length()); } @@ -3058,7 +3165,7 @@ class TokenPartitionTreesEqualPredFormatTest TEST_F(TokenPartitionTreesEqualPredFormatTest, EqualityCheck) { using TPT = TokenPartitionTreeBuilder; - static const auto ref_tree = + static const TokenPartitionTree ref_tree = TPT(PartitionPolicyEnum::kAlwaysExpand, { TPT(2, {0, 2}, PartitionPolicyEnum::kFitOnLineElseExpand), @@ -3151,5 +3258,38 @@ TEST_F(TokenPartitionTreesEqualPredFormatTest, EqualityCheck) { } } +TEST(TokenPartitionTreeTest, Misc) { + { + auto partition = TokenPartitionTree(); + Visit(Overload{ + [](UnwrappedLineNode&) { SUCCEED(); }, + [](TokenPartitionChoiceNode&) { ADD_FAILURE(); }, + [](auto&) { ADD_FAILURE(); }, + }, + partition); + } + { + auto partition = TokenPartitionTree(TokenPartitionTree::Kind::kPartition); + Visit(Overload{ + [](UnwrappedLineNode&) { SUCCEED(); }, + [](TokenPartitionChoiceNode&) { ADD_FAILURE(); }, + [](auto&) { ADD_FAILURE(); }, + }, + partition); + } + { + auto partition = TokenPartitionTree(TokenPartitionTree::Kind::kChoice); + Visit(Overload{ + [](TokenPartitionChoiceNode&) { SUCCEED(); }, + [](UnwrappedLineNode&) { ADD_FAILURE(); }, + [](auto&) { ADD_FAILURE(); }, + }, + partition); + } + + // TODO: deep copy constructor + // TODO: move constructor +} + } // namespace } // namespace verible diff --git a/common/formatting/token_partition_tree_test_utils.cc b/common/formatting/token_partition_tree_test_utils.cc index 02fc8be0cc..e526f943bf 100644 --- a/common/formatting/token_partition_tree_test_utils.cc +++ b/common/formatting/token_partition_tree_test_utils.cc @@ -39,15 +39,15 @@ TokenPartitionTree TokenPartitionTreeBuilder::build( const std::vector& tokens) const { TokenPartitionTree node; - node.Children().reserve(children_.size()); + node.Children()->reserve(children_.size()); for (const auto& child : children_) { - node.Children().push_back(child.build(tokens)); + node.Children()->push_back(child.build(tokens)); } FormatTokenRange node_tokens; if (token_indexes_range_.first < 0) { - const auto& child_nodes = node.Children(); - CHECK(!child_nodes.empty()); + CHECK(!is_leaf(node)); + const auto& child_nodes = *node.Children(); CHECK_LT(token_indexes_range_.second, 0); node_tokens.set_begin(child_nodes.front().Value().TokensRange().begin()); node_tokens.set_end(child_nodes.back().Value().TokensRange().end()); diff --git a/common/formatting/tree_unwrapper.cc b/common/formatting/tree_unwrapper.cc index a1e46d0cf6..a228896d10 100644 --- a/common/formatting/tree_unwrapper.cc +++ b/common/formatting/tree_unwrapper.cc @@ -46,7 +46,7 @@ static TokenPartitionTree MakeInitialUnwrappedLines( auto unwrapped_lines = TokenPartitionTree(UnwrappedLine( indentation, first_token, PartitionPolicyEnum::kAlwaysExpand)); // First unwrapped line - unwrapped_lines.Children().emplace_back(UnwrappedLine( + unwrapped_lines.Children()->emplace_back(UnwrappedLine( indentation, first_token // TODO(fangism): set partition policy here? )); @@ -64,7 +64,7 @@ TreeUnwrapper::TreeUnwrapper( // The "top-most" UnwrappedLine spans the entire file, so the first // unwrapped line should be a considered a partition (child) thereof. // This acts like 'pushing' the first child onto a stack. - active_unwrapped_lines_(&unwrapped_lines_.Children().front()) { + active_unwrapped_lines_(&unwrapped_lines_.Children()->front()) { // Every new unwrapped line will be initially empty, but the range // will point to the correct starting position in the preformatted_tokens_ // array, and be able to 'extend' into the array of preformatted_tokens_. @@ -150,7 +150,7 @@ const UnwrappedLine& TreeUnwrapper::CurrentUnwrappedLine() const { } void TreeUnwrapper::RemoveTrailingEmptyPartitions(TokenPartitionTree* node) { - auto& children = node->Children(); + auto& children = *node->Children(); while (!children.empty() && children.back().Value().IsEmpty()) { children.pop_back(); } @@ -159,7 +159,7 @@ void TreeUnwrapper::RemoveTrailingEmptyPartitions(TokenPartitionTree* node) { void TreeUnwrapper::CloseUnwrappedLineTreeNode( TokenPartitionTree* node, preformatted_tokens_type::const_iterator token_iter) { - const auto& children = node->Children(); + const auto& children = *node->Children(); if (!children.empty()) { const auto last_child_end = children.back().Value().TokensRange().end(); CHECK(last_child_end >= token_iter) @@ -203,7 +203,7 @@ void TreeUnwrapper::StartNewUnwrappedLine(PartitionPolicyEnum partitioning, // maintained through re-use of an existing node. if (!is_leaf(*active_unwrapped_lines_)) { VLOG(4) << "removed pre-existing child partitions."; - active_unwrapped_lines_->Children().clear(); + active_unwrapped_lines_->Children()->clear(); } } else { // To maintain the invariant that a parent range's upper-bound is equal @@ -214,7 +214,7 @@ void TreeUnwrapper::StartNewUnwrappedLine(PartitionPolicyEnum partitioning, FinishUnwrappedLine(); // Create new sibling to current unwrapped line, maintaining same level. - auto& siblings = active_unwrapped_lines_->Parent()->Children(); + auto& siblings = *active_unwrapped_lines_->Parent()->Children(); siblings.emplace_back(UnwrappedLine(current_indentation_spaces_, CurrentFormatTokenIterator(), partitioning)); @@ -297,11 +297,11 @@ TreeUnwrapper::VisitIndentedChildren(const SyntaxTreeNode& node, StartNewUnwrappedLine(partitioning, &node); // Start first child right away. - active_unwrapped_lines_->Children().emplace_back( + active_unwrapped_lines_->Children()->emplace_back( UnwrappedLine(current_indentation_spaces_, CurrentFormatTokenIterator(), PartitionPolicyEnum::kFitOnLineElseExpand /* default */)); const ValueSaver tree_saver( - &active_unwrapped_lines_, &active_unwrapped_lines_->Children().back()); + &active_unwrapped_lines_, &active_unwrapped_lines_->Children()->back()); VLOG(3) << __FUNCTION__ << ", new child node " << NodePath(*active_unwrapped_lines_) << ": " << CurrentUnwrappedLine(); diff --git a/common/util/tree_operations_test.cc b/common/util/tree_operations_test.cc index 63569c21b8..71e52ac409 100644 --- a/common/util/tree_operations_test.cc +++ b/common/util/tree_operations_test.cc @@ -384,9 +384,13 @@ TEST(Misc, TreeNodeTraits) { TYPED_TEST(SimpleNodeTest, StandaloneFunctionsInterface) { if constexpr (std::is_const_v) { - EXPECT_TRUE(std::is_const_vroot))>>); + EXPECT_TRUE( + std::is_const_v< + std::remove_reference_troot))>>); } else { - EXPECT_FALSE(std::is_const_vroot))>>); + EXPECT_FALSE( + std::is_const_v< + std::remove_reference_troot))>>); } } diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc index 4c3a873bbe..614c7a4e15 100644 --- a/verilog/formatting/tree_unwrapper.cc +++ b/verilog/formatting/tree_unwrapper.cc @@ -1593,8 +1593,8 @@ static void AttachSeparatorToPreviousOrNextPartition( void AttachSeparatorsToListElementPartitions(TokenPartitionTree* partition) { CHECK_NOTNULL(partition); // Skip the first partition, it can't contain just a separator. - for (int i = 1; i < static_cast(partition->Children().size()); ++i) { - auto& subpartition = partition->Children()[i]; + for (int i = 1; i < static_cast(partition->Children()->size()); ++i) { + auto& subpartition = partition->Children()->at(i); // This can change children count AttachSeparatorToPreviousOrNextPartition(&subpartition); } @@ -1637,10 +1637,10 @@ static void AttachTrailingSemicolonToPreviousPartition( static void AdjustSubsequentPartitionsIndentation(TokenPartitionTree* partition, int indentation) { // Adjust indentation of subsequent partitions - const auto npartitions = partition->Children().size(); + const auto npartitions = partition->Children()->size(); if (npartitions > 1) { - const auto& first_partition = partition->Children().front(); - const auto& last_partition = partition->Children().back(); + const auto& first_partition = partition->Children()->front(); + const auto& last_partition = partition->Children()->back(); // Do not indent intentionally wrapped partitions, e.g. // { (>>[assign foo = {] } @@ -1656,7 +1656,7 @@ static void AdjustSubsequentPartitionsIndentation(TokenPartitionTree* partition, !(PartitionEndsWithOpenParen(first_partition) && PartitionIsCloseParenSemi(last_partition))) { for (unsigned int idx = 1; idx < npartitions; ++idx) { - AdjustIndentationRelative(&partition->Children()[idx], indentation); + AdjustIndentationRelative(&partition->Children()->at(idx), indentation); } } } @@ -1666,11 +1666,11 @@ static void AdjustSubsequentPartitionsIndentation(TokenPartitionTree* partition, // from the subpartition following it. static void AttachOpeningBraceToDeclarationsAssignmentOperator( TokenPartitionTree* partition) { - const int children_count = partition->Children().size(); + const int children_count = partition->Children()->size(); if (children_count <= 1) return; for (int i = 0; i < children_count - 1; ++i) { - auto& current = RightmostDescendant(partition->Children()[i]); + auto& current = RightmostDescendant(partition->Children()->at(i)); const auto tokens = current.Value().TokensRange(); if (tokens.empty()) continue; @@ -1686,7 +1686,7 @@ static void AttachOpeningBraceToDeclarationsAssignmentOperator( last_non_comment_it->TokenEnum() == ':')) continue; - const auto& next = partition->Children()[i + 1]; + const auto& next = partition->Children()->at(i + 1); if (next.Value().IsEmpty()) continue; const auto& next_token = next.Value().TokensRange().front(); @@ -1714,8 +1714,8 @@ static void ReshapeIfClause(const SyntaxTreeNode& node, } // Then fuse the 'begin' partition with the preceding 'if (...)' - auto& if_body_partition = partition.Children().back(); - auto& begin_partition = if_body_partition.Children().front(); + auto& if_body_partition = partition.Children()->back(); + auto& begin_partition = if_body_partition.Children()->front(); verible::MergeLeafIntoPreviousLeaf(&begin_partition); partition.Value().SetPartitionPolicy( if_body_partition.Value().PartitionPolicy()); @@ -1738,7 +1738,7 @@ static void ReshapeElseClause(const SyntaxTreeNode& node, // Then fuse 'else' and 'begin' partitions together // or fuse the 'else' and 'if' (header) partitions together - auto& children = partition.Children(); + auto& children = *partition.Children(); auto else_partition_iter = std::find_if( children.begin(), children.end(), [](const TokenPartitionTree& n) { const auto* origin = n.Value().Origin(); @@ -1781,7 +1781,7 @@ static void PushEndIntoElsePartition(TokenPartitionTree* partition_ptr) { // Do not flatten, so that if- and else- clauses can make formatting // decisions independently from each other. auto& partition = *partition_ptr; - auto& if_clause_partition = partition.Children().front(); + auto& if_clause_partition = partition.Children()->front(); auto* end_partition = &RightmostDescendant(if_clause_partition); auto* end_parent = verible::MergeLeafIntoNextLeaf(end_partition); // if moving leaf results in any singleton partitions, hoist. @@ -1820,7 +1820,7 @@ static void FlattenElseIfElse(const SyntaxTreeNode& else_clause, const auto& else_body_subnode = *GetAnyControlStatementBody(else_clause); if (NodeIsConditionalConstruct(else_body_subnode) && GetAnyConditionalElseClause(else_body_subnode) != nullptr) { - FlattenOneChild(partition, partition.Children().size() - 1); + FlattenOneChild(partition, partition.Children()->size() - 1); } } @@ -1842,7 +1842,8 @@ static bool TokenIsComment(const PreFormatToken& t) { static void SetCommentLinePartitionAsAlreadyFormatted( TokenPartitionTree* partition) { - for (auto& child : partition->Children()) { + if (is_leaf(*partition)) return; + for (auto& child : *partition->Children()) { if (!is_leaf(child)) continue; const auto tokens = child.Value().TokensRange(); if (std::all_of(tokens.begin(), tokens.end(), TokenIsComment)) { @@ -1892,9 +1893,9 @@ struct ContainsToken { static const TokenPartitionTree* FindDirectChild( const TokenPartitionTree* parent, TokenPartitionPredicate predicate) { if (!parent) return nullptr; - auto iter = std::find_if(parent->Children().begin(), parent->Children().end(), - std::move(predicate)); - if (iter == parent->Children().end()) return nullptr; + auto iter = std::find_if(parent->Children()->begin(), + parent->Children()->end(), std::move(predicate)); + if (iter == parent->Children()->end()) return nullptr; return &(*iter); } @@ -1909,7 +1910,8 @@ static TokenPartitionTree* FindDirectChild(TokenPartitionTree* parent, static bool LineBreaksInsidePartitionBeforeChild( const TokenPartitionTree& parent, const TokenPartitionTree& child) { - for (auto& node : parent.Children()) { + CHECK(!is_leaf(parent)); + for (auto& node : *parent.Children()) { if (PartitionIsForcedIntoNewLine(node)) return true; if (&node == &child) break; } @@ -1958,7 +1960,7 @@ class MacroCallReshaper { main_node_origin->Tag() == NodeTag(NodeEnum::kMacroCall)) { // '(' must follow macro identifier in the same line CHECK(!PartitionIsForcedIntoNewLine(*paren_group_)); - identifier_with_paren_group->Children().clear(); + identifier_with_paren_group->Children()->clear(); identifier_with_paren_group->Value().SetPartitionPolicy( PartitionPolicyEnum::kAlreadyFormatted); } @@ -1978,8 +1980,9 @@ class MacroCallReshaper { MoveRightParenToArgumentList(); } - if (argument_list_ && style_.try_wrap_long_lines) { - for (auto& arg : argument_list_->Children()) { + if (argument_list_ && style_.try_wrap_long_lines && + !is_leaf(*argument_list_)) { + for (auto& arg : *argument_list_->Children()) { const auto tokens = arg.Value().TokensRange(); // Hack: in order to avoid wrapping just before comment, do not enable // wrapping on partitions containing comments. @@ -2001,18 +2004,18 @@ class MacroCallReshaper { } else if (!LineBreaksInsidePartitionBeforeChild(*paren_group_, *l_paren_)) { GroupIdentifierCommentsAndLeftParen(); - } else if (l_paren_ != &paren_group_->Children().front()) { + } else if (l_paren_ != &paren_group_->Children()->front()) { GroupCommentsAndLeftParen(); } if (argument_list_) { HoistOnlyChildPartition(argument_list_); - if (paren_group_->Children().size() == 3) { + if (paren_group_->Children()->size() == 3) { // Children: '(', argument_list, ')' GroupLeftParenAndArgumentList(); } - if (!PartitionIsForcedIntoNewLine(paren_group_->Children().back())) { + if (!PartitionIsForcedIntoNewLine(paren_group_->Children()->back())) { paren_group_->Value().SetPartitionPolicy( PartitionPolicyEnum::kJuxtapositionOrIndentedStack); } else { @@ -2076,7 +2079,7 @@ class MacroCallReshaper { } if (!IsLastChild(*paren_group_)) { - semicolon_ = &main_node_->Children().back(); + semicolon_ = &main_node_->Children()->back(); if (!ContainsToken{';'}(*semicolon_)) { LOG_BUG(LOG(ERROR), "Unexpected partition(s) after the call."); LOG(ERROR) << "\n" << *main_node_; @@ -2097,7 +2100,7 @@ class MacroCallReshaper { LOG_BUG(LOG(ERROR), "'(' not found."); return false; } - r_paren_ = &paren_group_->Children().back(); + r_paren_ = &paren_group_->Children()->back(); if (!ContainsToken{ ')', verilog_tokentype::MacroCallCloseToEndLine}(*r_paren_)) { LOG_BUG(LOG(ERROR), "')' not found."); @@ -2110,12 +2113,12 @@ class MacroCallReshaper { #undef LOG_BUG bool ReshapeEmptyParenGroup() { - if (paren_group_->Children().size() == 2 && l_paren_ != r_paren_ && + if (paren_group_->Children()->size() == 2 && l_paren_ != r_paren_ && l_paren_->Value().TokensRange().end() == r_paren_->Value().TokensRange().begin() && !PartitionIsForcedIntoNewLine(*r_paren_)) { VLOG(6) << "Flatten paren group."; - paren_group_->Children().clear(); + paren_group_->Children()->clear(); } if (is_leaf(*paren_group_)) { if (!PartitionIsForcedIntoNewLine(*paren_group_)) { @@ -2137,17 +2140,17 @@ class MacroCallReshaper { PartitionPolicyEnum::kWrap)); group.Value().SpanUpToToken(r_paren_->Value().TokensRange().begin()); - const auto paren_group_begin = paren_group_->Children().begin(); + const auto paren_group_begin = paren_group_->Children()->begin(); const auto args_begin = paren_group_begin + BirthRank(*l_paren_) + 1; const auto args_end = paren_group_begin + BirthRank(*r_paren_); // Move partitions into the group. - group.Children().assign(std::make_move_iterator(args_begin), - std::make_move_iterator(args_end)); - for (auto& node : group.Children()) { + group.Children()->assign(std::make_move_iterator(args_begin), + std::make_move_iterator(args_end)); + for (auto& node : *group.Children()) { verible::AdjustIndentationAbsolute(&node, arguments_indentation); } // Remove leftover entries of all grouped partitions except the first. - paren_group_->Children().erase(args_begin + 1, args_end); + paren_group_->Children()->erase(args_begin + 1, args_end); // Move the group into first grouped partition's place. *args_begin = std::move(group); @@ -2161,12 +2164,12 @@ class MacroCallReshaper { r_paren_->Value().SetIndentationSpaces( argument_list_->Value().IndentationSpaces()); const auto r_paren_iter = - paren_group_->Children().begin() + BirthRank(*r_paren_); + paren_group_->Children()->begin() + BirthRank(*r_paren_); argument_list_->Value().SpanUpToToken( r_paren_->Value().TokensRange().end()); - argument_list_->Children().push_back(std::move(*r_paren_)); - r_paren_ = &argument_list_->Children().back(); - paren_group_->Children().erase(r_paren_iter); + argument_list_->Children()->push_back(std::move(*r_paren_)); + r_paren_ = &argument_list_->Children()->back(); + paren_group_->Children()->erase(r_paren_iter); } if (!PartitionIsForcedIntoNewLine(*r_paren_)) { // We want to avoid wrapping just before `)`. It would be best to use @@ -2182,7 +2185,7 @@ class MacroCallReshaper { })) { // Merge verible::MergeLeafIntoPreviousLeaf(r_paren_); - r_paren_ = &argument_list_->Children().back(); + r_paren_ = &argument_list_->Children()->back(); } else { // Group auto& last_argument = *PreviousSibling(*r_paren_); @@ -2191,12 +2194,13 @@ class MacroCallReshaper { last_argument.Value().TokensRange().begin(), PartitionPolicyEnum::kJuxtaposition)); group.Value().SpanUpToToken(r_paren_->Value().TokensRange().end()); - group.Children().reserve(2); - group.Children().push_back(std::move(last_argument)); - group.Children().push_back(std::move(*r_paren_)); - argument_list_->Children().erase(argument_list_->Children().end() - 1); - argument_list_->Children().back() = std::move(group); - r_paren_ = &argument_list_->Children().back(); + group.Children()->reserve(2); + group.Children()->push_back(std::move(last_argument)); + group.Children()->push_back(std::move(*r_paren_)); + argument_list_->Children()->erase(argument_list_->Children()->end() - + 1); + argument_list_->Children()->back() = std::move(group); + r_paren_ = &argument_list_->Children()->back(); } } } @@ -2215,42 +2219,42 @@ class MacroCallReshaper { paren_group_->Value().SpanBackToToken(group.Value().TokensRange().begin()); const auto old_identifier_iter = - identifier_->Parent()->Children().begin() + BirthRank(*identifier_); + identifier_->Parent()->Children()->begin() + BirthRank(*identifier_); const auto l_paren_index = BirthRank(*l_paren_); auto nodes_to_group = verible::iterator_range( - paren_group_->Children().begin(), - paren_group_->Children().begin() + l_paren_index + 1); + paren_group_->Children()->begin(), + paren_group_->Children()->begin() + l_paren_index + 1); const auto nodes_count = l_paren_index + 1 + 1; // + 1 for identifier - group.Children().reserve(nodes_count); + group.Children()->reserve(nodes_count); // Move partitions into the group. - group.Children().push_back(std::move(*identifier_)); - group.Children().insert(group.Children().end(), - std::make_move_iterator(nodes_to_group.begin()), - std::make_move_iterator(nodes_to_group.end())); + group.Children()->push_back(std::move(*identifier_)); + group.Children()->insert(group.Children()->end(), + std::make_move_iterator(nodes_to_group.begin()), + std::make_move_iterator(nodes_to_group.end())); // Remove leftover entries of all grouped partitions except: // * First paren_group's child: will be reused as a group node. // * The identifier: removing it will invalidate iterators; done later. - paren_group_->Children().erase(nodes_to_group.begin() + 1, - nodes_to_group.end()); + paren_group_->Children()->erase(nodes_to_group.begin() + 1, + nodes_to_group.end()); // Move the group into first grouped partition's place. *nodes_to_group.begin() = std::move(group); // Remove identifier - main_node_->Children().erase(old_identifier_iter); + main_node_->Children()->erase(old_identifier_iter); HoistOnlyChildPartition(main_node_); paren_group_ = main_node_; identifier_ = nullptr; - l_paren_ = &paren_group_->Children().front(); + l_paren_ = &paren_group_->Children()->front(); if (argument_list_) { argument_list_ = NextSibling(*l_paren_); - r_paren_ = r_paren_is_in_arg_list ? &argument_list_->Children().back() + r_paren_ = r_paren_is_in_arg_list ? &argument_list_->Children()->back() : NextSibling(*argument_list_); } else { - r_paren_ = &paren_group_->Children().back(); + r_paren_ = &paren_group_->Children()->back(); } } @@ -2265,13 +2269,13 @@ class MacroCallReshaper { paren_group_ = main_node_; identifier_ = nullptr; - l_paren_ = &paren_group_->Children().front(); + l_paren_ = &paren_group_->Children()->front(); if (argument_list_) { argument_list_ = NextSibling(*l_paren_); - r_paren_ = r_paren_is_in_arg_list ? &argument_list_->Children().back() + r_paren_ = r_paren_is_in_arg_list ? &argument_list_->Children()->back() : NextSibling(*argument_list_); } else { - r_paren_ = &paren_group_->Children().back(); + r_paren_ = &paren_group_->Children()->back(); } } @@ -2284,18 +2288,18 @@ class MacroCallReshaper { group.Value().SpanUpToToken(l_paren_->Value().TokensRange().end()); const auto l_paren_index = BirthRank(*l_paren_); - const auto paren_group_begin = paren_group_->Children().begin(); + const auto paren_group_begin = paren_group_->Children()->begin(); auto nodes_to_group = verible::iterator_range( paren_group_begin, paren_group_begin + l_paren_index + 1); // Move partitions into the group. - group.Children().assign(std::make_move_iterator(nodes_to_group.begin()), - std::make_move_iterator(nodes_to_group.end())); + group.Children()->assign(std::make_move_iterator(nodes_to_group.begin()), + std::make_move_iterator(nodes_to_group.end())); // Remove leftover entries of all grouped partitions except: // * First paren_group's child: will be reused as a group node. - paren_group_->Children().erase(nodes_to_group.begin() + 1, - nodes_to_group.end()); + paren_group_->Children()->erase(nodes_to_group.begin() + 1, + nodes_to_group.end()); // Move the group into first grouped partition's place. *nodes_to_group.begin() = std::move(group); @@ -2304,16 +2308,16 @@ class MacroCallReshaper { if (argument_list_) { argument_list_ = NextSibling(*l_paren_); - r_paren_ = is_nested_ ? &argument_list_->Children().back() + r_paren_ = is_nested_ ? &argument_list_->Children()->back() : NextSibling(*argument_list_); } else { - r_paren_ = &paren_group_->Children().back(); + r_paren_ = &paren_group_->Children()->back(); } } void GroupLeftParenAndArgumentList() { auto old_argument_list_iter = - paren_group_->Children().begin() + BirthRank(*argument_list_); + paren_group_->Children()->begin() + BirthRank(*argument_list_); const auto group_policy = PartitionIsForcedIntoNewLine(*argument_list_) @@ -2327,7 +2331,7 @@ class MacroCallReshaper { AdoptSubtree(group, std::move(*l_paren_), std::move(*argument_list_)); *l_paren_ = std::move(group); - paren_group_->Children().erase(old_argument_list_iter); + paren_group_->Children()->erase(old_argument_list_iter); l_paren_ = nullptr; argument_list_ = nullptr; @@ -2469,8 +2473,8 @@ void TreeUnwrapper::ReshapeTokenPartitions( case NodeEnum::kDataDeclaration: { AttachTrailingSemicolonToPreviousPartition(&partition); auto& data_declaration_partition = partition; - auto& children = data_declaration_partition.Children(); - CHECK(!children.empty()); + CHECK(!is_leaf(data_declaration_partition)); + auto& children = *data_declaration_partition.Children(); // TODO(fangism): fuse qualifiers (if any) with type partition @@ -2564,7 +2568,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( return (sym->Kind() == verible::SymbolKind::kLeaf); })) { auto& assigned_value_partition = - data_declaration_partition.Children()[1]; + data_declaration_partition.Children()->at(1); partition.Value().SetPartitionPolicy( PartitionPolicyEnum::kAppendFittingSubPartitions); assigned_value_partition.Value().SetPartitionPolicy( @@ -2589,7 +2593,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( // [function ... ] (task header/prototype) // Push the "import..." down. { - verible::MergeLeafIntoNextLeaf(&partition.Children().front()); + verible::MergeLeafIntoNextLeaf(&partition.Children()->front()); AttachTrailingSemicolonToPreviousPartition(&partition); break; } @@ -2599,7 +2603,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( // Take advantage here that preceding data declaration partition // was already shaped. auto& target_instance_partition = partition; - auto& children = target_instance_partition.Children(); + auto& children = *target_instance_partition.Children(); // Attach ')' to the instance name verible::MergeLeafIntoNextLeaf(PreviousSibling(children.back())); @@ -2613,8 +2617,8 @@ void TreeUnwrapper::ReshapeTokenPartitions( } case NodeEnum::kModuleHeader: { // Allow empty ports to appear as "();" - if (partition.Children().size() >= 2) { - auto& last = partition.Children().back(); + if (partition.Children()->size() >= 2) { + auto& last = partition.Children()->back(); auto& last_prev = *ABSL_DIE_IF_NULL(PreviousSibling(last)); if (PartitionStartsWithCloseParen(last) && PartitionEndsWithOpenParen(last_prev)) { @@ -2624,7 +2628,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( // If there were any parameters or ports at all, expand. // TODO(fangism): This should be done by inspecting the CST node, // instead of the partition structure. - if (partition.Children().size() > 2) { + if (partition.Children()->size() > 2) { partition.Value().SetPartitionPolicy( PartitionPolicyEnum::kAlwaysExpand); } @@ -2633,8 +2637,8 @@ void TreeUnwrapper::ReshapeTokenPartitions( case NodeEnum::kClassHeader: { // Allow empty parameters to appear as "#();" - if (partition.Children().size() >= 2) { - auto& last = partition.Children().back(); + if (partition.Children()->size() >= 2) { + auto& last = partition.Children()->back(); auto& last_prev = *ABSL_DIE_IF_NULL(PreviousSibling(last)); if (PartitionStartsWithCloseParen(last) && PartitionEndsWithOpenParen(last_prev)) { @@ -2758,19 +2762,19 @@ void TreeUnwrapper::ReshapeTokenPartitions( // TODO(fangism): This should be done smarter (using CST) or removed // after better handling of function calls inside expressions // e.g. kBinaryExpression, kUnaryPrefixExpression... - if (partition.Children().size() > 1) { + if (partition.Children()->size() > 1) { auto if_header_partition_iter = std::find_if( - partition.Children().begin(), partition.Children().end(), + partition.Children()->begin(), partition.Children()->end(), [](const TokenPartitionTree& n) { const auto* origin = n.Value().Origin(); return origin && origin->Tag() == verible::LeafTag(verilog_tokentype::TK_if); }); - if (if_header_partition_iter == partition.Children().end()) break; + if (if_header_partition_iter == partition.Children()->end()) break; // Adjust indentation of all partitions following if header recursively for (auto& child : make_range(if_header_partition_iter + 1, - partition.Children().end())) { + partition.Children()->end())) { verible::AdjustIndentationRelative(&child, style.wrap_spaces); } } @@ -2828,7 +2832,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( case NodeEnum::kConstraintDeclaration: { // TODO(fangism): kConstraintSet should be handled similarly with {} - if (partition.Children().size() == 2) { + if (partition.Children()->size() == 2) { auto& last = RightmostDescendant(partition); if (PartitionIsCloseBrace(last)) { verible::MergeLeafIntoPreviousLeaf(&last); @@ -2866,7 +2870,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( VLOG(4) << "before moving semicolon:\n" << partition; AttachTrailingSemicolonToPreviousPartition(&partition); // RHS may have been further partitioned, e.g. a macro call. - auto& children = partition.Children(); + auto& children = *partition.Children(); if (children.size() == 2 && verible::is_leaf(children.front()) /* left side */) { verible::MergeLeafIntoNextLeaf(&children.front()); @@ -2898,7 +2902,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( AttachSeparatorsToListElementPartitions(&partition); // Merge the 'assign' keyword with the (first) x=y assignment. // TODO(fangism): reshape for multiple assignments. - verible::MergeLeafIntoNextLeaf(&partition.Children().front()); + verible::MergeLeafIntoNextLeaf(&partition.Children()->front()); VLOG(4) << "after merging 'assign':\n" << partition; AdjustSubsequentPartitionsIndentation(&partition, style.wrap_spaces); VLOG(4) << "after adjusting partitions indentation:\n" << partition; @@ -2908,7 +2912,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( // This applies to loop statements and loop generate constructs. // There are two 'partitions' with ';'. // Merge those with their predecessor sibling partitions. - auto& children = partition.Children(); + auto& children = *partition.Children(); const auto iter1 = std::find_if(children.begin(), children.end(), PartitionStartsWithSemicolon); CHECK(iter1 != children.end()); @@ -2952,7 +2956,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( (++token_tmp)->TokenEnum() == TK_EOL_COMMENT && (++token_tmp)->TokenEnum() == TK_begin) { AdjustIndentationRelative( - &partition.Children()[(partition.Children().size() - 1)], + &partition.Children()->at(partition.Children()->size() - 1), style.indentation_spaces); break; } @@ -2961,7 +2965,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( // with the preceding keyword or header partition. if (NodeIsBeginEndBlock( verible::SymbolCastToNode(*node.children().back()))) { - auto& seq_block_partition = partition.Children().back(); + auto& seq_block_partition = partition.Children()->back(); VLOG(4) << "block partition: " << seq_block_partition; auto& begin_partition = LeftmostDescendant(seq_block_partition); VLOG(4) << "begin partition: " << begin_partition; @@ -2978,7 +2982,7 @@ void TreeUnwrapper::ReshapeTokenPartitions( if (const auto* dw = GetDoWhileStatementBody(node); dw != nullptr && NodeIsBeginEndBlock(*dw)) { // between do... and while (...); - auto& seq_block_partition = partition.Children()[1]; + auto& seq_block_partition = partition.Children()->at(1); // merge "do" <- "begin" auto& begin_partition = LeftmostDescendant(seq_block_partition); @@ -3000,9 +3004,9 @@ void TreeUnwrapper::ReshapeTokenPartitions( ->MatchesTagAnyOf({NodeEnum::kProceduralTimingControlStatement, NodeEnum::kSeqBlock})) { // Merge 'always' keyword with next sibling, and adjust subtree indent. - verible::MergeLeafIntoNextLeaf(&partition.Children().front()); + verible::MergeLeafIntoNextLeaf(&partition.Children()->front()); verible::AdjustIndentationAbsolute( - &partition.Children().front(), + &partition.Children()->front(), partition.Value().IndentationSpaces()); VLOG(4) << "after merging 'always':\n" << partition; } @@ -3061,8 +3065,8 @@ void TreeUnwrapper::ReshapeTokenPartitions( } case NodeEnum::kPatternExpression: { - if (partition.Children().size() >= 3) { - auto& colon = partition.Children()[1]; + if (partition.Children()->size() >= 3) { + auto& colon = partition.Children()->at(1); AttachSeparatorToPreviousOrNextPartition(&colon); } FlattenOnlyChildrenWithChildren(partition); diff --git a/verilog/formatting/tree_unwrapper_test.cc b/verilog/formatting/tree_unwrapper_test.cc index 22f2b52dd2..afe8f1aa5c 100644 --- a/verilog/formatting/tree_unwrapper_test.cc +++ b/verilog/formatting/tree_unwrapper_test.cc @@ -206,7 +206,7 @@ bool VerifyUnwrappedLines(std::ostream* stream, *stream << "expected:\n" << *diff.right << std::endl; *stream << "but got :\n" << verible::TokenPartitionTreePrinter(*diff.left) << std::endl; - const auto left_children = diff.left->Children().size(); + const auto left_children = diff.left->Children()->size(); const auto right_children = diff.right->Children().size(); EXPECT_EQ(left_children, right_children) << "code:\n" << test_case.source_code;