Merge pull request chipsalliance#1045 from antmicro/mglb/UseLayoutOpt…

…imizerForFunctionCalls

Use Layout Optimizer to format nested function calls

common/formatting/BUILD

@@ -93,6 +93,7 @@ cc_library(
         "//common/text:concrete_syntax_leaf",
         "//common/text:token_info",
         "//common/util:container_iterator_range",
+        "//common/util:iterator_adaptors",
         "//common/util:logging",
         "//common/util:spacer",
         "@com_google_absl//absl/base:core_headers",
@@ -120,6 +121,7 @@ cc_library(
         "layout_optimizer.cc",
         "layout_optimizer_internal.h",
     ],
+    hdrs = ["layout_optimizer.h"],
     deps = [
         ":basic_format_style",
         ":line_wrap_searcher",

common/formatting/align.cc

@@ -805,27 +805,6 @@ static std::vector<DeferredTokenAlignment> ComputeAlignedRowSpacings(
   return align_actions;
 }
 
-// Given a const_iterator and the original mutable container, return
-// the corresponding mutable iterator (without resorting to const_cast).
-// The 'Container' type is not deducible from function arguments alone.
-// TODO(fangism): provide this from common/util/iterator_adaptors.
-template <class Container>
-typename Container::iterator ConvertToMutableIterator(
-    typename Container::const_iterator const_iter,
-    typename Container::iterator base) {
-  const typename Container::const_iterator cbase(base);
-  return base + std::distance(cbase, const_iter);
-}
-
-static MutableFormatTokenRange ConvertToMutableFormatTokenRange(
-    const FormatTokenRange& const_range,
-    MutableFormatTokenRange::iterator base) {
-  using array_type = std::vector<PreFormatToken>;
-  return MutableFormatTokenRange(
-      ConvertToMutableIterator<array_type>(const_range.begin(), base),
-      ConvertToMutableIterator<array_type>(const_range.end(), base));
-}
-
 static const AlignmentRow* RightmostSubcolumnWithTokens(
     const AlignmentRow& node) {
   if (!node.Value().tokens.empty()) return &node;
@@ -861,8 +840,7 @@ static void CommitAlignmentDecisionToRow(
     }
     // Tag every subtree as having already been committed to alignment.
     partition->ApplyPostOrder([](TokenPartitionTree& node) {
-      node.Value().SetPartitionPolicy(
-          PartitionPolicyEnum::kSuccessfullyAligned);
+      node.Value().SetPartitionPolicy(PartitionPolicyEnum::kAlreadyFormatted);
     });
   }
 }

common/formatting/format_token.cc

@@ -23,6 +23,7 @@
 #include "common/strings/display_utils.h"
 #include "common/strings/range.h"
 #include "common/text/token_info.h"
+#include "common/util/iterator_adaptors.h"
 #include "common/util/logging.h"
 #include "common/util/spacer.h"
 
@@ -297,4 +298,12 @@ void PreserveSpacesOnDisabledTokenRanges(
   }
 }
 
+MutableFormatTokenRange ConvertToMutableFormatTokenRange(
+    const FormatTokenRange& const_range,
+    MutableFormatTokenRange::iterator base) {
+  return MutableFormatTokenRange(
+      ConvertToMutableIterator(const_range.begin(), base),
+      ConvertToMutableIterator(const_range.end(), base));
+}
+
 }  // namespace verible

common/formatting/format_token.h

@@ -175,6 +175,13 @@ using FormatTokenRange =
 using MutableFormatTokenRange =
     container_iterator_range<std::vector<PreFormatToken>::iterator>;
 
+// Given a const_range and a mutable iterator to the original mutable container,
+// return the corresponding mutable iterator range (without resorting to
+// const_cast).
+MutableFormatTokenRange ConvertToMutableFormatTokenRange(
+    const FormatTokenRange& const_range,
+    MutableFormatTokenRange::iterator base);
+
 // Enumeration for the final decision about spacing between tokens.
 // Related enum: SpacingConstraint.
 // These values are also used during line wrap searching and optimization.

common/formatting/layout_optimizer.cc

@@ -12,9 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Improve code formatting by optimizing token partitions layout with
-// algorithm documented in https://research.google/pubs/pub44667/
-// (similar tool for R language: https://github.com/google/rfmt)
+// Implementation of a code layout optimizer described by
+// Phillip Yelland in "A New Approach to Optimal Code Formatting"
+// (https://research.google/pubs/pub44667/) and originally implemented
+// in rfmt (https://github.com/google/rfmt).
+
+#include "common/formatting/layout_optimizer.h"
 
 #include <algorithm>
 #include <iomanip>
@@ -30,6 +33,97 @@
 
 namespace verible {
 
+void OptimizeTokenPartitionTree(const BasicFormatStyle& style,
+                                TokenPartitionTree* node,
+                                std::vector<PreFormatToken>* ftokens) {
+  CHECK_NOTNULL(node);
+
+  VLOG(4) << __FUNCTION__ << ", before:\n" << *node;
+  const auto indentation = node->Value().IndentationSpaces();
+
+  LayoutFunctionFactory factory(style);
+
+  const std::function<LayoutFunction(const TokenPartitionTree&)> TraverseTree =
+      [&TraverseTree, &style, &factory](const TokenPartitionTree& subnode) {
+        const auto policy = subnode.Value().PartitionPolicy();
+
+        if (subnode.is_leaf()) {
+          return factory.Line(subnode.Value());
+        }
+
+        switch (policy) {
+          case PartitionPolicyEnum::kOptimalFunctionCallLayout: {
+            // Support only function/macro/system calls for now
+            CHECK_EQ(subnode.Children().size(), 2);
+
+            const auto& function_header = subnode.Children()[0];
+            const auto& function_args = subnode.Children()[1];
+
+            auto header = TraverseTree(function_header);
+            auto args = TraverseTree(function_args);
+
+            auto stack_layout = factory.Stack({
+                header,
+                factory.Indent(args, style.wrap_spaces),
+            });
+            if (args.MustWrap()) {
+              return stack_layout;
+            }
+            auto juxtaposed_layout = factory.Juxtaposition({
+                header,
+                args,
+            });
+            return factory.Choice({
+                std::move(juxtaposed_layout),
+                std::move(stack_layout),
+            });
+          }
+
+          case PartitionPolicyEnum::kAppendFittingSubPartitions:
+          case PartitionPolicyEnum::kFitOnLineElseExpand: {
+            absl::FixedArray<LayoutFunction> layouts(subnode.Children().size());
+            std::transform(subnode.Children().begin(), subnode.Children().end(),
+                           layouts.begin(), TraverseTree);
+            return factory.Wrap(layouts.begin(), layouts.end());
+          }
+
+          case PartitionPolicyEnum::kAlwaysExpand:
+          case PartitionPolicyEnum::kTabularAlignment: {
+            absl::FixedArray<LayoutFunction> layouts(subnode.Children().size());
+            std::transform(subnode.Children().begin(), subnode.Children().end(),
+                           layouts.begin(), TraverseTree);
+            return factory.Stack(layouts.begin(), layouts.end());
+          }
+
+            // TODO(mglb): Think about introducing PartitionPolicies that
+            // correspond directly to combinators in LayoutFunctionFactory.
+            // kOptimalFunctionCallLayout strategy could then be implemented
+            // directly in TreeUnwrapper. It would also allow for proper
+            // handling of other policies (e.g. kTabularAlignment) in subtrees.
+
+          default: {
+            LOG(FATAL) << "Unsupported policy: " << policy << "\n"
+                       << "Node:\n"
+                       << subnode;
+            return LayoutFunction();
+          }
+        }
+      };
+
+  const LayoutFunction layout_function = TraverseTree(*node);
+  CHECK(!layout_function.empty());
+  VLOG(4) << __FUNCTION__ << ", layout function:\n" << layout_function;
+
+  auto iter = layout_function.AtOrToTheLeftOf(indentation);
+  CHECK(iter != layout_function.end());
+  VLOG(4) << __FUNCTION__ << ", layout:\n" << iter->layout;
+
+  TreeReconstructor tree_reconstructor(indentation, style);
+  tree_reconstructor.TraverseTree(iter->layout);
+  tree_reconstructor.ReplaceTokenPartitionTreeNode(node, ftokens);
+  VLOG(4) << __FUNCTION__ << ", after:\n" << *node;
+}
+
 namespace {
 
 // Adopts sublayouts of 'source' into 'destination' if 'source' and
@@ -405,4 +499,110 @@ LayoutFunction LayoutFunctionFactory::Choice(
   return result;
 }
 
+void TreeReconstructor::TraverseTree(const LayoutTree& layout_tree) {
+  const auto relative_indentation = layout_tree.Value().IndentationSpaces();
+  const ValueSaver<int> indent_saver(
+      &current_indentation_spaces_,
+      current_indentation_spaces_ + relative_indentation);
+  // Setting indentation for a line that is going to be appended is invalid and
+  // probably has been done for some reason that is not going to work as
+  // intended.
+  LOG_IF(WARNING,
+         ((relative_indentation > 0) && (active_unwrapped_line_ != nullptr)))
+      << "Discarding indentation of a line that's going to be appended.";
+
+  switch (layout_tree.Value().Type()) {
+    case LayoutType::kLine: {
+      CHECK(layout_tree.Children().empty());
+      if (active_unwrapped_line_ == nullptr) {
+        auto uwline = layout_tree.Value().ToUnwrappedLine();
+        uwline.SetIndentationSpaces(current_indentation_spaces_);
+        // Prevent SearchLineWraps from processing optimized lines.
+        uwline.SetPartitionPolicy(PartitionPolicyEnum::kAlreadyFormatted);
+        active_unwrapped_line_ = &unwrapped_lines_.emplace_back(uwline);
+      } else {
+        const auto tokens = layout_tree.Value().ToUnwrappedLine().TokensRange();
+        active_unwrapped_line_->SpanUpToToken(tokens.end());
+      }
+      return;
+    }
+
+    case LayoutType::kJuxtaposition: {
+      // Append all children
+      for (const auto& child : layout_tree.Children()) {
+        TraverseTree(child);
+      }
+      return;
+    }
+
+    case LayoutType::kStack: {
+      if (layout_tree.Children().empty()) {
+        return;
+      }
+      if (layout_tree.Children().size() == 1) {
+        TraverseTree(layout_tree.Children().front());
+        return;
+      }
+
+      // Calculate indent for 2nd and further lines.
+      int indentation = current_indentation_spaces_;
+      if (active_unwrapped_line_ != nullptr) {
+        indentation = FitsOnLine(*active_unwrapped_line_, style_).final_column +
+                      layout_tree.Value().SpacesBefore();
+      }
+
+      // Append first child
+      TraverseTree(layout_tree.Children().front());
+
+      // Put remaining children in their own (indented) lines
+      const ValueSaver<int> indent_saver(&current_indentation_spaces_,
+                                         indentation);
+      for (const auto& child : make_range(layout_tree.Children().begin() + 1,
+                                          layout_tree.Children().end())) {
+        active_unwrapped_line_ = nullptr;
+        TraverseTree(child);
+      }
+      return;
+    }
+  }
+}
+
+void TreeReconstructor::ReplaceTokenPartitionTreeNode(
+    TokenPartitionTree* node, std::vector<PreFormatToken>* ftokens) const {
+  CHECK_NOTNULL(node);
+  CHECK_NOTNULL(ftokens);
+  CHECK(!unwrapped_lines_.empty());
+
+  const auto& first_line = unwrapped_lines_.front();
+  const auto& last_line = unwrapped_lines_.back();
+
+  node->Value() = UnwrappedLine(first_line);
+  node->Value().SpanUpToToken(last_line.TokensRange().end());
+  node->Value().SetIndentationSpaces(current_indentation_spaces_);
+  node->Value().SetPartitionPolicy(
+      PartitionPolicyEnum::kOptimalFunctionCallLayout);
+
+  node->Children().clear();
+  for (auto& uwline : unwrapped_lines_) {
+    if (!uwline.IsEmpty()) {
+      auto line_ftokens = ConvertToMutableFormatTokenRange(uwline.TokensRange(),
+                                                           ftokens->begin());
+
+      // Discard first token's original spacing (the partition has already
+      // proper indentation set).
+      line_ftokens.front().before.break_decision = SpacingOptions::MustWrap;
+      line_ftokens.front().before.spaces_required = 0;
+      line_ftokens.pop_front();
+
+      for (auto& line_ftoken : line_ftokens) {
+        SpacingOptions& decision = line_ftoken.before.break_decision;
+        if (decision == SpacingOptions::Undecided) {
+          decision = SpacingOptions::MustAppend;
+        }
+      }
+    }
+    node->AdoptSubtree(uwline);
+  }
+}
+
 }  // namespace verible

common/formatting/layout_optimizer.h

@@ -0,0 +1,81 @@
+// Copyright 2017-2021 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Implementation of a code layout optimizer described by
+// Phillip Yelland in "A New Approach to Optimal Code Formatting"
+// (https://research.google/pubs/pub44667/) and originally implemented
+// in rfmt (https://github.com/google/rfmt).
+
+#ifndef VERIBLE_VERILOG_FORMATTING_LAYOUT_OPTIMIZER_H_
+#define VERIBLE_VERILOG_FORMATTING_LAYOUT_OPTIMIZER_H_
+
+#include <algorithm>
+#include <iterator>
+#include <ostream>
+#include <type_traits>
+#include <vector>
+
+#include "absl/container/fixed_array.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
+#include "common/formatting/basic_format_style.h"
+#include "common/formatting/token_partition_tree.h"
+#include "common/formatting/unwrapped_line.h"
+#include "common/util/vector_tree.h"
+
+namespace verible {
+
+// Handles formatting of TokenPartitionTree 'node' that uses
+// PartitionPolicyEnum::kOptimalFunctionCallLayout partition policy.
+// 'ftokens' is used to get mutable iterators to tokens of formatted partitions.
+// The function changes only tokens that are spanned by the passed partitions
+// tree.
+//
+// It is designed to format function calls and requires specific partition
+// tree structure:
+//
+//   <function call node, policy: kOptimalFunctionCallLayout> {
+//     <function header> { ... },
+//     <function arguments> { ... }
+//   }
+//
+// Nested kOptimalFunctionCallLayout partitions are supported.
+//
+// EXAMPLE INPUT TREE
+//
+// Code: `uvm_info(`gfn, $sformatf("%0d %0d\n", cfg.num_pulses, i), UVM_DEBUG)
+//
+// Partition tree:
+// { (>>[...], policy: optimal-function-call-layout) // call:
+//   { (>>[`uvm_info (]) }                           // - header
+//   { (>>>>>>[...])                                 // - arguments:
+//     { (>>>>>>[`gfn ,]) }                          //   - (arg)
+//     { (>>>>>>[...], policy: optimal-function-call-layout) // nested call:
+//       { (>>>>>>[$sformatf (]) }                           // - header
+//       { (>>>>>>>>>>[...])                                 // - arguments
+//         { (>>>>>>>>>>["%0d %0d\n" ,]) }                   //   - (arg)
+//         { (>>>>>>>>>>[cfg . num_pulses ,]) }              //   - (arg)
+//         { (>>>>>>>>>>[i ) ,]) }                           //   - (arg)
+//       }
+//     }
+//     { (>>>>>>[UVM_DEBUG )]) }                     //   - (arg)
+//   }
+// }
+void OptimizeTokenPartitionTree(const BasicFormatStyle& style,
+                                TokenPartitionTree* node,
+                                std::vector<PreFormatToken>* ftokens);
+
+}  // namespace verible
+
+#endif  // VERIBLE_VERILOG_FORMATTING_LAYOUT_OPTIMIZER_H_