Skip to content

Commit

Permalink
[WIP] Convert TokenPartitionTree into class hierarchy wrapper.
Browse files Browse the repository at this point in the history 
TODO:
- Extract Choice-related class and code to a separate commit
- Documentation
- Tests
- Cleanup
  • Loading branch information
@mglb
mglb committed Apr 30, 2022
1 parent 439bd07 commit 7d77d45
Show file tree
Hide file tree
Showing 13 changed files with 1,222 additions and 428 deletions.
1 change: 1 addition & 0 deletions common/formatting/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 14 additions & 14 deletions common/formatting/align.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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());
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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());
Expand Down
30 changes: 15 additions & 15 deletions common/formatting/align_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<TaggedTokenPartitionRange> ranges(
GetPartitionAlignmentSubranges(children, [](const TokenPartitionTree&
Expand Down Expand Up @@ -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<TaggedTokenPartitionRange> ranges(
GetPartitionAlignmentSubranges(children, &PartitionSelector));
Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down
58 changes: 29 additions & 29 deletions common/formatting/layout_optimizer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -500,7 +500,7 @@ LayoutFunction TokenPartitionsLayoutOptimizer::CalculateOptimalLayout(

// Traverse and calculate children layouts

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

switch (node.Value().PartitionPolicy()) {
case PartitionPolicyEnum::kJuxtaposition:
Expand All @@ -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);
});
Expand All @@ -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;
Expand Down Expand Up @@ -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 "
Expand All @@ -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;

Expand All @@ -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());
Expand All @@ -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;
Expand All @@ -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());
Expand Down Expand Up @@ -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));
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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_);
}
}

Expand Down

0 comments on commit 7d77d45

Please sign in to comment.