Skip to content

Commit 73947e5

Browse files
author
Mariusz Glebocki
committed
[WIP] Convert TokenPartitionTree into class hierarchy wrapper.
TODO: - Extract Choice-related class and code to a separate commit - Documentation - Tests - Cleanup
1 parent ecf8662 commit 73947e5

13 files changed

+1222
-428
lines changed

‎common/formatting/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ cc_library(
173173
":format_token",
174174
":line_wrap_searcher",
175175
":unwrapped_line",
176+
"//common/util:variant",
176177
"//common/strings:display_utils",
177178
"//common/strings:position",
178179
"//common/strings:range",

‎common/formatting/align.cc‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -983,7 +983,7 @@ void AlignablePartitionGroup::ApplyAlignment(
983983
const GroupAlignmentData& align_data) const {
984984
auto row = alignable_rows_.begin();
985985
for (const auto& align_actions : align_data.align_actions_2D) {
986-
(*row)->Children().clear();
986+
(*row)->Children()->clear();
987987
VLOG(3) << __FUNCTION__ << " processing row: " << **row;
988988
if (!align_actions.empty()) {
989989
auto& node = **row;
@@ -994,9 +994,9 @@ void AlignablePartitionGroup::ApplyAlignment(
994994

995995
verible::TokenPartitionTree* current_cell = nullptr;
996996
if (align_actions.front().ftoken != ftokens.begin()) {
997-
node.Children().emplace_back(
997+
node.Children()->emplace_back(
998998
UnwrappedLine(0, ftokens.begin(), PartitionPolicyEnum::kInline));
999-
current_cell = &node.Children().back();
999+
current_cell = &node.Children()->back();
10001000
}
10011001

10021002
for (const auto& action : align_actions) {
@@ -1007,10 +1007,10 @@ void AlignablePartitionGroup::ApplyAlignment(
10071007
<< StringSpanOfTokenRange(current_cell->Value().TokensRange())
10081008
<< " ]";
10091009
}
1010-
node.Children().emplace_back(
1010+
node.Children()->emplace_back(
10111011
UnwrappedLine(action.new_before_spacing, action.ftoken,
10121012
PartitionPolicyEnum::kInline));
1013-
current_cell = &node.Children().back();
1013+
current_cell = &node.Children()->back();
10141014
}
10151015
if (current_cell) {
10161016
current_cell->Value().SpanUpToToken(ftokens.end());
@@ -1140,7 +1140,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) {
11401140
VLOG(4) << "partition before:\n"
11411141
<< TokenPartitionTreePrinter(partition, true);
11421142

1143-
partition.Children().clear();
1143+
partition.Children()->clear();
11441144
auto tokens = partition.Value().TokensRange();
11451145
if (tokens.empty()) {
11461146
partition.Value().SetPartitionPolicy(
@@ -1157,7 +1157,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) {
11571157

11581158
auto line = UnwrappedLine(indentation, tokens.begin(),
11591159
PartitionPolicyEnum::kAlreadyFormatted);
1160-
partition.Children().emplace_back(line);
1160+
partition.Children()->emplace_back(line);
11611161

11621162
if (tokens.size() > 1) {
11631163
// First token
@@ -1167,7 +1167,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) {
11671167
auto slice =
11681168
UnwrappedLine(0, tokens.begin(), PartitionPolicyEnum::kInline);
11691169
slice.SpanNextToken();
1170-
partition.Children().back().Children().emplace_back(slice);
1170+
partition.Children()->back().Children()->emplace_back(slice);
11711171

11721172
// Remaining tokens
11731173
for (auto it = tokens.begin() + 1; it != tokens.end(); ++it) {
@@ -1180,7 +1180,7 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) {
11801180
std::size_t last_newline_pos = whitespace.find_last_of('\n');
11811181
if (last_newline_pos != absl::string_view::npos) {
11821182
// Update end of current line.
1183-
partition.Children().back().Value().SpanUpToToken(it);
1183+
partition.Children()->back().Value().SpanUpToToken(it);
11841184
// Start a new line.
11851185
// Newlines count does not matter here. All newlines in leading
11861186
// whitespace of the first token in a line are always preserved.
@@ -1192,19 +1192,19 @@ void FormatUsingOriginalSpacing(TokenPartitionRange partition_range) {
11921192
// indentation + (this line orig. indent) - (1st line orig. indent)
11931193
const auto line =
11941194
UnwrappedLine(0, it, PartitionPolicyEnum::kAlreadyFormatted);
1195-
partition.Children().emplace_back(line);
1195+
partition.Children()->emplace_back(line);
11961196
// Count only spaces after the last '\n'.
11971197
spacing -= last_newline_pos + 1;
11981198
}
11991199

12001200
auto slice = UnwrappedLine(spacing, it, PartitionPolicyEnum::kInline);
12011201
slice.SpanNextToken();
1202-
partition.Children().back().Children().emplace_back(slice);
1202+
partition.Children()->back().Children()->emplace_back(slice);
12031203
}
12041204
}
1205-
partition.Children().back().Value().SpanUpToToken(tokens.end());
1205+
partition.Children()->back().Value().SpanUpToToken(tokens.end());
12061206

1207-
if (partition.Children().size() == 1) {
1207+
if (partition.Children()->size() == 1) {
12081208
HoistOnlyChild(partition);
12091209
} else {
12101210
partition.Value().SetPartitionPolicy(PartitionPolicyEnum::kAlwaysExpand);
@@ -1271,7 +1271,7 @@ void TabularAlignTokens(
12711271
// possibly some other ignored element like comments.
12721272

12731273
auto& partition = *partition_ptr;
1274-
auto& subpartitions = partition.Children();
1274+
auto& subpartitions = *partition.Children();
12751275
// Identify groups of partitions to align, separated by blank lines.
12761276
const TokenPartitionRange subpartitions_range(subpartitions.begin(),
12771277
subpartitions.end());

‎common/formatting/align_test.cc‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class MatrixTreeAlignmentTestFixture : public AlignmentTestFixture {
162162

163163
std::string Render() {
164164
std::ostringstream stream;
165-
for (auto& child : partition_.Children()) {
165+
for (auto& child : *partition_.Children()) {
166166
const auto policy = child.Value().PartitionPolicy();
167167
if (policy == PartitionPolicyEnum::kAlreadyFormatted) {
168168
ApplyAlreadyFormattedPartitionPropertiesToTokens(&child,
@@ -333,7 +333,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, OneInterTokenPaddingWithIndent) {
333333
ftoken.before.spaces_required = 1;
334334
}
335335
// Indent each partition.
336-
for (auto& child : partition_.Children()) {
336+
for (auto& child : *partition_.Children()) {
337337
child.Value().SetIndentationSpaces(4);
338338
}
339339

@@ -354,7 +354,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, IgnoreCommentLine) {
354354
}
355355
// Leave the 'commented' line indented.
356356
pre_format_tokens_[2].before.break_decision = SpacingOptions::MustWrap;
357-
partition_.Children()[1].Value().SetIndentationSpaces(1);
357+
partition_.Children()->at(1).Value().SetIndentationSpaces(1);
358358

359359
// Pretend lines that begin with "three" are to be ignored, like comments.
360360
auto ignore_threes = [](const TokenPartitionTree& partition) {
@@ -406,7 +406,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, CompletelyDisabledNoAlignmentWithIndent) {
406406
for (auto& ftoken : pre_format_tokens_) {
407407
ftoken.before.spaces_required = 1;
408408
}
409-
for (auto& child : partition_.Children()) {
409+
for (auto& child : *partition_.Children()) {
410410
child.Value().SetIndentationSpaces(3);
411411
}
412412
pre_format_tokens_[0].before.break_decision = SpacingOptions::MustWrap;
@@ -442,7 +442,7 @@ TEST_F(Sparse3x3MatrixAlignmentMoreSpacesTest,
442442
for (auto& ftoken : pre_format_tokens_) {
443443
ftoken.before.spaces_required = 1;
444444
}
445-
for (auto& child : partition_.Children()) {
445+
for (auto& child : *partition_.Children()) {
446446
child.Value().SetIndentationSpaces(1);
447447
}
448448
pre_format_tokens_[0].before.break_decision = SpacingOptions::MustWrap;
@@ -510,7 +510,7 @@ TEST_F(Sparse3x3MatrixAlignmentTest, DisabledByColumnLimitIndented) {
510510
for (auto& ftoken : pre_format_tokens_) {
511511
ftoken.before.spaces_required = 1;
512512
}
513-
for (auto& child : partition_.Children()) {
513+
for (auto& child : *partition_.Children()) {
514514
child.Value().SetIndentationSpaces(3);
515515
}
516516

@@ -592,7 +592,7 @@ class MultiAlignmentGroupTest : public AlignmentTestFixture {
592592
std::ostringstream stream;
593593
int position = 0;
594594
const absl::string_view text(sample_);
595-
for (auto& child : partition_.Children()) {
595+
for (auto& child : *partition_.Children()) {
596596
const auto policy = child.Value().PartitionPolicy();
597597
if (policy == PartitionPolicyEnum::kAlreadyFormatted) {
598598
ApplyAlreadyFormattedPartitionPropertiesToTokens(&child,
@@ -717,8 +717,8 @@ class GetPartitionAlignmentSubrangesTestFixture : public AlignmentTestFixture {
717717
};
718718

719719
TEST_F(GetPartitionAlignmentSubrangesTestFixture, VariousRanges) {
720-
const TokenPartitionRange children(partition_.Children().begin(),
721-
partition_.Children().end());
720+
const TokenPartitionRange children(partition_.Children()->begin(),
721+
partition_.Children()->end());
722722

723723
const std::vector<TaggedTokenPartitionRange> ranges(
724724
GetPartitionAlignmentSubranges(children, [](const TokenPartitionTree&
@@ -820,8 +820,8 @@ class GetPartitionAlignmentSubrangesSubtypedTestFixture
820820
};
821821

822822
TEST_F(GetPartitionAlignmentSubrangesSubtypedTestFixture, VariousRanges) {
823-
const TokenPartitionRange children(partition_.Children().begin(),
824-
partition_.Children().end());
823+
const TokenPartitionRange children(partition_.Children()->begin(),
824+
partition_.Children()->end());
825825

826826
const std::vector<TaggedTokenPartitionRange> ranges(
827827
GetPartitionAlignmentSubranges(children, &PartitionSelector));
@@ -1058,7 +1058,7 @@ class SubcolumnsTreeAlignmentTest : public MatrixTreeAlignmentTestFixture {
10581058
}
10591059
uwline.SpanUpToToken(token_iter);
10601060
uwline.SetOrigin(item.get());
1061-
partition_.Children().emplace_back(std::move(uwline));
1061+
partition_.Children()->emplace_back(std::move(uwline));
10621062
SymbolCastToNode(*syntax_tree_).AppendChild(std::move(item));
10631063
}
10641064
}
@@ -1185,7 +1185,7 @@ TEST_F(SubcolumnsTreeAlignmentTest, OneInterTokenPaddingExceptFront) {
11851185
ftoken.before.spaces_required = 1;
11861186
}
11871187
// Find first token of each line and require 0 spaces before them.
1188-
for (auto& line : partition_.Children()) {
1188+
for (auto& line : *partition_.Children()) {
11891189
const auto tokens = line.Value().TokensRange();
11901190
if (!tokens.empty()) {
11911191
const PreFormatToken& front = tokens.front();
@@ -1232,7 +1232,7 @@ TEST_F(SubcolumnsTreeAlignmentTest,
12321232
for (auto& ftoken : pre_format_tokens_) {
12331233
ftoken.before.spaces_required = 1;
12341234
}
1235-
for (auto& line : partition_.Children()) {
1235+
for (auto& line : *partition_.Children()) {
12361236
line.Value().SetIndentationSpaces(2);
12371237
}
12381238

@@ -1262,7 +1262,7 @@ class MultiSubcolumnsTreeAlignmentTest : public SubcolumnsTreeAlignmentTest {
12621262
std::ostringstream stream;
12631263
int position = 0;
12641264
const absl::string_view text(sample_);
1265-
for (auto& child : partition_.Children()) {
1265+
for (auto& child : *partition_.Children()) {
12661266
const auto policy = child.Value().PartitionPolicy();
12671267
if (policy == PartitionPolicyEnum::kAlreadyFormatted) {
12681268
ApplyAlreadyFormattedPartitionPropertiesToTokens(&child,

‎common/formatting/layout_optimizer.cc‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ int AlreadyFormattedPartitionLength(const TokenPartitionTree& partition) {
9090
width += token.before.spaces_required + token.Length();
9191
}
9292

93-
for (const auto& child : partition.Children()) {
93+
for (const auto& child : *partition.Children()) {
9494
CHECK_EQ(child.Value().PartitionPolicy(), PartitionPolicyEnum::kInline);
9595
if (child.Value().TokensRange().begin() != tokens.begin()) {
9696
const auto& first_token = child.Value().TokensRange().front();
@@ -500,7 +500,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
500500

501501
// Traverse and calculate children layouts
502502

503-
absl::FixedArray<LayoutFunction> layouts(node.Children().size());
503+
absl::FixedArray<LayoutFunction> layouts(node.Children()->size());
504504

505505
switch (node.Value().PartitionPolicy()) {
506506
case PartitionPolicyEnum::kJuxtaposition:
@@ -509,7 +509,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
509509
case PartitionPolicyEnum::kFitOnLineElseExpand:
510510
case PartitionPolicyEnum::kAppendFittingSubPartitions:
511511
case PartitionPolicyEnum::kJuxtapositionOrIndentedStack: {
512-
std::transform(node.Children().begin(), node.Children().end(),
512+
std::transform(node.Children()->begin(), node.Children()->end(),
513513
layouts.begin(), [=](const TokenPartitionTree& n) {
514514
return this->CalculateOptimalLayout(n);
515515
});
@@ -520,7 +520,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
520520
case PartitionPolicyEnum::kAlwaysExpand:
521521
case PartitionPolicyEnum::kTabularAlignment: {
522522
const int indentation = node.Value().IndentationSpaces();
523-
std::transform(node.Children().begin(), node.Children().end(),
523+
std::transform(node.Children()->begin(), node.Children()->end(),
524524
layouts.begin(), [=](const TokenPartitionTree& n) {
525525
const int relative_indentation =
526526
n.Value().IndentationSpaces() - indentation;
@@ -558,10 +558,11 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
558558
case PartitionPolicyEnum::kStack:
559559
return factory_.Stack(layouts.begin(), layouts.end());
560560
case PartitionPolicyEnum::kWrap: {
561-
if (VLOG_IS_ON(0) && node.Children().size() > 2) {
562-
const int indentation = node.Children()[1].Value().IndentationSpaces();
563-
for (const auto& child : iterator_range(node.Children().begin() + 2,
564-
node.Children().end())) {
561+
if (VLOG_IS_ON(0) && node.Children()->size() > 2) {
562+
const int indentation =
563+
node.Children()->at(1).Value().IndentationSpaces();
564+
for (const auto& child : iterator_range(node.Children()->begin() + 2,
565+
node.Children()->end())) {
565566
if (child.Value().IndentationSpaces() != indentation) {
566567
VLOG(0) << "Indentations of subpartitions from the second to the "
567568
"last are not equal. Using indentation of the second "
@@ -571,8 +572,8 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
571572
}
572573
}
573574
const int hanging_indentation =
574-
(node.Children().size() > 1)
575-
? (node.Children()[1].Value().IndentationSpaces() -
575+
(node.Children()->size() > 1)
576+
? (node.Children()->at(1).Value().IndentationSpaces() -
576577
node.Value().IndentationSpaces())
577578
: 0;
578579

@@ -592,7 +593,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
592593
const int indentation = node.Value().IndentationSpaces();
593594
for (size_t i = 0; i < layouts.size(); ++i) {
594595
const int relative_indentation =
595-
node.Children()[i].Value().IndentationSpaces() - indentation;
596+
node.Children()->at(i).Value().IndentationSpaces() - indentation;
596597
layouts[i] = factory_.Indent(layouts[i], relative_indentation);
597598
}
598599
auto stack = factory_.Stack(layouts.begin(), layouts.end());
@@ -615,7 +616,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
615616
// When not a leaf, it contains partitions with kInline
616617
// policy. Pack them horizontally.
617618
const bool all_children_are_inlines =
618-
std::all_of(node.Children().begin(), node.Children().end(),
619+
std::all_of(node.Children()->begin(), node.Children()->end(),
619620
[](const TokenPartitionTree& child) {
620621
return child.Value().PartitionPolicy() ==
621622
PartitionPolicyEnum::kInline;
@@ -631,7 +632,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(
631632
// Preserve spacing of the first sublayout. This has to be done because
632633
// the first layout in a line uses IndentationSpaces instead of
633634
// SpacesBefore.
634-
const auto indent = node.Children().front().Value().IndentationSpaces();
635+
const auto indent = node.Children()->front().Value().IndentationSpaces();
635636
layouts.front() = factory_.Indent(layouts.front(), indent);
636637

637638
return factory_.Juxtaposition(layouts.begin(), layouts.end());
@@ -679,26 +680,26 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
679680
layout.TokensRange().begin(),
680681
PartitionPolicyEnum::kAlreadyFormatted);
681682
uwline.SpanUpToToken(layout.TokensRange().end());
682-
tree_.Children().emplace_back(uwline);
683-
current_node_ = &tree_.Children().back();
683+
tree_.Children()->emplace_back(uwline);
684+
current_node_ = &tree_.Children()->back();
684685
} else {
685686
const auto tokens = layout.TokensRange();
686687
CHECK(current_node_->Value().TokensRange().end() == tokens.begin());
687688

688689
current_node_->Value().SpanUpToToken(tokens.end());
689690

690-
auto& slices = current_node_->Children();
691+
auto& slices = *current_node_->Children();
691692
// TODO(mglb): add support for break_decision == Preserve
692693
if (layout.SpacesBefore() == tokens.front().before.spaces_required) {
693694
// No need for separate inline partition
694-
if (!slices.empty())
695+
if (!is_leaf(*current_node_))
695696
slices.back().Value().SpanUpToToken(tokens.end());
696697
return;
697698
}
698699

699700
// Wrap previous tokens in the line
700-
if (slices.empty()) {
701-
current_node_->Children().emplace_back(
701+
if (is_leaf(*current_node_)) {
702+
current_node_->Children()->emplace_back(
702703
UnwrappedLine(0, current_node_->Value().TokensRange().begin(),
703704
PartitionPolicyEnum::kInline));
704705
}
@@ -708,7 +709,7 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
708709
auto slice = UnwrappedLine(layout.SpacesBefore(), tokens.begin(),
709710
PartitionPolicyEnum::kInline);
710711
slice.SpanUpToToken(tokens.end());
711-
current_node_->Children().emplace_back(slice);
712+
current_node_->Children()->emplace_back(slice);
712713
}
713714
return;
714715
}
@@ -756,20 +757,19 @@ void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
756757
void TreeReconstructor::ReplaceTokenPartitionTreeNode(
757758
TokenPartitionTree* node) {
758759
CHECK_NOTNULL(node);
759-
CHECK(!tree_.Children().empty());
760+
CHECK(!is_leaf(tree_));
760761

761-
if (tree_.Children().size() == 1) {
762-
*node = std::move(tree_.Children().front());
762+
if (tree_.Children()->size() == 1) {
763+
*node = std::move(tree_.Children()->front());
763764
} else {
764-
const auto& first_line = tree_.Children().front().Value();
765-
const auto& last_line = tree_.Children().back().Value();
765+
const auto& first_line = tree_.Children()->front().Value();
766+
const auto& last_line = tree_.Children()->back().Value();
766767

767-
node->Value() = UnwrappedLine(current_indentation_spaces_,
768+
tree_.Value() = UnwrappedLine(current_indentation_spaces_,
768769
first_line.TokensRange().begin(),
769770
PartitionPolicyEnum::kAlwaysExpand);
770-
node->Value().SpanUpToToken(last_line.TokensRange().end());
771-
node->Children().clear();
772-
AdoptSubtreesFrom(*node, &tree_);
771+
tree_.Value().SpanUpToToken(last_line.TokensRange().end());
772+
*node = std::move(tree_);
773773
}
774774
}
775775

0 commit comments

Comments
 (0)