From 629d8908d05d895ca1586e98bd73284ad41b77f4 Mon Sep 17 00:00:00 2001 From: David Fang Date: Thu, 21 May 2020 14:12:07 -0700 Subject: [PATCH] Rewrite SequenceStreamFormatter to work with C++11. Tested with: bazel test -c opt --cxxopt=-std=c++11 //... PiperOrigin-RevId: 312738516 --- common/strings/display_utils.h | 29 +++++++++----- common/strings/display_utils_test.cc | 6 ++- common/text/tree_context_visitor.cc | 5 +++ common/text/tree_context_visitor.h | 6 +-- verilog/formatting/BUILD | 1 + verilog/formatting/align.cc | 56 ++++++++-------------------- 6 files changed, 49 insertions(+), 54 deletions(-) diff --git a/common/strings/display_utils.h b/common/strings/display_utils.h index ee5514fe1..f3102cbc2 100644 --- a/common/strings/display_utils.h +++ b/common/strings/display_utils.h @@ -37,7 +37,10 @@ struct AutoTruncate { std::ostream& operator<<(std::ostream&, const AutoTruncate& trunc); -namespace internal { +// TODO(fangism): once C++17 becomes the minimum standard for building +// push the following block into an internal namespace, and use auto +// return types instead of directly rnaming these types. +// namespace internal { // Helper struct for bundling parameters to absl::StrJoin. // This is useful for contructing printer adapters for types that @@ -65,7 +68,7 @@ std::ostream& operator<<(std::ostream& stream, << t.suffix; } -} // namespace internal +// } // namespace internal // SequenceFormatter helps create custom formatters (pretty-printers) for // standard container types, when providing a plain std::ostream& operator<< @@ -75,11 +78,18 @@ std::ostream& operator<<(std::ostream& stream, // // Example usage (define the following for your specific container type): // Suppose MySequenceType is a typedef to a container like std::list. -// Define a lambda (implicit return type, w/o auto-return type supported): +// Define a forwarding function: // -// constexpr auto MySequenceFormatter = [](const MySequenceType& t) { +// [pre C++17] +// SequenceStreamFormatter MySequenceFormatter( +// const MySequenceType& t) { // return verible::SequenceFormatter(t, " | ", "< ", " >"); -// }; +// } +// +// [C++17 and higher, supporting auto return type]: +// auto MySequenceFormatter(const MySequenceType& t) { +// return verible::SequenceFormatter(t, " | ", "< ", " >"); +// } // // and call it: // stream << MySequenceFormatter(sequence_obj) << ...; @@ -88,10 +98,11 @@ std::ostream& operator<<(std::ostream& stream, // "< 1 | 2 | 3 | ... >" // template -internal::SequenceStreamFormatter SequenceFormatter( - const T& t, absl::string_view sep = ", ", absl::string_view prefix = "", - absl::string_view suffix = "") { - return internal::SequenceStreamFormatter{t, sep, prefix, suffix}; +SequenceStreamFormatter SequenceFormatter(const T& t, + absl::string_view sep = ", ", + absl::string_view prefix = "", + absl::string_view suffix = "") { + return SequenceStreamFormatter{t, sep, prefix, suffix}; } } // namespace verible diff --git a/common/strings/display_utils_test.cc b/common/strings/display_utils_test.cc index e05237919..441f63766 100644 --- a/common/strings/display_utils_test.cc +++ b/common/strings/display_utils_test.cc @@ -55,9 +55,11 @@ typedef std::vector IntVector; // Normally a definition like the following would appear in a header // to be shared. -static constexpr auto AngleBracketFormatter = [](const IntVector& t) { +// TODO(fangism): Use auto return type with C++17 as minimum standard. +static SequenceStreamFormatter AngleBracketFormatter( + const IntVector& t) { return SequenceFormatter(t, " | ", "< ", " >"); -}; +} TEST(SequenceFormatterTest, AngleBracketVectorNotation) { const IntVector v{5, 6, 7, 8}; diff --git a/common/text/tree_context_visitor.cc b/common/text/tree_context_visitor.cc index d9f416233..9416b432a 100644 --- a/common/text/tree_context_visitor.cc +++ b/common/text/tree_context_visitor.cc @@ -49,6 +49,11 @@ void TreeContextPathVisitor::Visit(const SyntaxTreeNode& node) { } } +SequenceStreamFormatter TreePathFormatter( + const SyntaxTreePath& path) { + return SequenceFormatter(path, ",", "[", "]"); +} + SyntaxTreePath NextSiblingPath(const SyntaxTreePath& path) { CHECK(!path.empty()); auto next = path; diff --git a/common/text/tree_context_visitor.h b/common/text/tree_context_visitor.h index 8f7cda120..7c76febb0 100644 --- a/common/text/tree_context_visitor.h +++ b/common/text/tree_context_visitor.h @@ -74,9 +74,9 @@ SyntaxTreePath NextSiblingPath(const SyntaxTreePath& path); // Format SyntaxTreePaths using: stream << TreePathFormatter(path); // It is necessary to define this way because SyntaxTreePath is a typedef to a // generic container type. -constexpr auto TreePathFormatter = [](const SyntaxTreePath& path) { - return SequenceFormatter(path, ",", "[", "]"); -}; +// TODO(fangism): Use auto return type once C++17 become the minimum standard. +SequenceStreamFormatter TreePathFormatter( + const SyntaxTreePath& path); } // namespace verible diff --git a/verilog/formatting/BUILD b/verilog/formatting/BUILD index 1754d8780..106d4baba 100644 --- a/verilog/formatting/BUILD +++ b/verilog/formatting/BUILD @@ -19,6 +19,7 @@ cc_library( "//common/formatting:format_token", "//common/formatting:token_partition_tree", "//common/formatting:unwrapped_line", + "//common/strings:display_utils", "//common/strings:position", "//common/strings:range", "//common/text:concrete_syntax_leaf", diff --git a/verilog/formatting/align.cc b/verilog/formatting/align.cc index 8d3970ba9..293606029 100644 --- a/verilog/formatting/align.cc +++ b/verilog/formatting/align.cc @@ -26,6 +26,7 @@ #include "absl/strings/str_join.h" #include "common/formatting/format_token.h" #include "common/formatting/unwrapped_line.h" +#include "common/strings/display_utils.h" #include "common/strings/range.h" #include "common/text/concrete_syntax_leaf.h" #include "common/text/concrete_syntax_tree.h" @@ -51,6 +52,8 @@ using verible::down_cast; using verible::FormatTokenRange; using verible::MutableFormatTokenRange; using verible::PreFormatToken; +using verible::SequenceFormatter; +using verible::SequenceStreamFormatter; using verible::Symbol; using verible::SymbolCastToNode; using verible::SyntaxTreeLeaf; @@ -59,6 +62,7 @@ using verible::SyntaxTreePath; using verible::TokenInfo; using verible::TokenPartitionTree; using verible::TreeContextPathVisitor; +using verible::TreePathFormatter; using verible::ValueSaver; template @@ -82,36 +86,6 @@ static bool IgnorePartition(const TokenPartitionTree& partition) { return false; } -template -struct SequenceStreamPrinter { - const T& sequence; - absl::string_view separator; - absl::string_view begin; - absl::string_view end; -}; - -template -std::ostream& operator<<(std::ostream& stream, - const SequenceStreamPrinter& t) { - return stream << t.begin - << absl::StrJoin(t.sequence.begin(), t.sequence.end(), - t.separator, absl::StreamFormatter()) - << t.end; -} - -template -SequenceStreamPrinter SequencePrinter(const T& t, - absl::string_view sep = ", ", - absl::string_view begin = "", - absl::string_view end = "") { - return SequenceStreamPrinter{t, sep, begin, end}; -} - -static SequenceStreamPrinter PathPrinter( - const SyntaxTreePath& path) { - return SequencePrinter(path, ",", "[", "]"); -} - using TokenPartitionIterator = std::vector::iterator; using TokenPartitionRange = verible::container_iterator_range; @@ -298,7 +272,7 @@ class ColumnSchemaScanner : public TreeContextPathVisitor { // When this occurs, take the (previous) leftmost token, and suppress // adding a new column. sparse_columns_.push_back(ColumnPositionEntry{path, leaf->get()}); - VLOG(2) << "reserving new column at " << PathPrinter(path); + VLOG(2) << "reserving new column at " << TreePathFormatter(path); } } @@ -368,7 +342,7 @@ class PortDeclarationColumnSchemaScanner : public ColumnSchemaScanner { void Visit(const SyntaxTreeNode& node) override { auto tag = NodeEnum(node.Tag().tag); VLOG(2) << __FUNCTION__ << ", node: " << tag << " at " - << PathPrinter(Path()); + << TreePathFormatter(Path()); switch (tag) { case NodeEnum::kPackedDimensions: { // Kludge: kPackedDimensions can appear in paths [2,1] and [2,0,2], @@ -413,7 +387,7 @@ class PortDeclarationColumnSchemaScanner : public ColumnSchemaScanner { void Visit(const SyntaxTreeLeaf& leaf) override { VLOG(2) << __FUNCTION__ << ", leaf: " << leaf.get() << " at " - << PathPrinter(Path()); + << TreePathFormatter(Path()); // TODO(b/70310743): subdivide and align '[' and ']' by giving them their // own single-character (sub) column. switch (leaf.get().token_enum) { @@ -460,9 +434,9 @@ class PortDeclarationColumnSchemaScanner : public ColumnSchemaScanner { } }; -static SequenceStreamPrinter MatrixRowPrinter( +static SequenceStreamFormatter MatrixRowFormatter( const AlignmentRow& row) { - return SequencePrinter(row, " | ", "<", ">"); + return SequenceFormatter(row, " | ", "<", ">"); } // Translate a sparse set of columns into a fully-populated matrix row. @@ -508,7 +482,8 @@ static void FillAlignmentRow( // Fill any sparse cells up to the last column. VLOG(3) << "fill up to last column"; const MutableFormatTokenRange empty_filler(token_end, token_end); - for (; last_column_index < column_positions.size(); ++last_column_index) { + for (const int n = column_positions.size(); last_column_index < n; + ++last_column_index) { VLOG(3) << "empty at column " << last_column_index; (*row)[last_column_index].tokens = empty_filler; } @@ -519,14 +494,15 @@ static void FillAlignmentRow( cell.tokens.set_end(upper_bound); upper_bound = cell.tokens.begin(); } - VLOG(2) << "end of " << __FUNCTION__ << ", row: " << MatrixRowPrinter(*row); + VLOG(2) << "end of " << __FUNCTION__ << ", row: " << MatrixRowFormatter(*row); } -struct MatrixCellSizePrinter { +struct MatrixCellSizeFormatter { const AlignmentMatrix& matrix; }; -std::ostream& operator<<(std::ostream& stream, const MatrixCellSizePrinter& p) { +std::ostream& operator<<(std::ostream& stream, + const MatrixCellSizeFormatter& p) { const AlignmentMatrix& matrix = p.matrix; for (auto& row : matrix) { stream << '[' @@ -548,7 +524,7 @@ static void ComputeCellWidths(AlignmentMatrix* matrix) { } } VLOG(2) << "end of " << __FUNCTION__ << ", cell sizes:\n" - << MatrixCellSizePrinter{*matrix}; + << MatrixCellSizeFormatter{*matrix}; } typedef std::vector AlignedFormattingColumnSchema;