Skip to content

Commit

Permalink
Rewrite SequenceStreamFormatter to work with C++11.
Browse files Browse the repository at this point in the history
Tested with: bazel test -c opt --cxxopt=-std=c++11 //...

PiperOrigin-RevId: 312738516
  • Loading branch information
fangism authored and hzeller committed May 21, 2020
1 parent 1d466c0 commit 629d890
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 54 deletions.
29 changes: 20 additions & 9 deletions common/strings/display_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<<
Expand All @@ -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<int>.
// 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<MySequenceType> 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) << ...;
Expand All @@ -88,10 +98,11 @@ std::ostream& operator<<(std::ostream& stream,
// "< 1 | 2 | 3 | ... >"
//
template <class T>
internal::SequenceStreamFormatter<T> SequenceFormatter(
const T& t, absl::string_view sep = ", ", absl::string_view prefix = "",
absl::string_view suffix = "") {
return internal::SequenceStreamFormatter<T>{t, sep, prefix, suffix};
SequenceStreamFormatter<T> SequenceFormatter(const T& t,
absl::string_view sep = ", ",
absl::string_view prefix = "",
absl::string_view suffix = "") {
return SequenceStreamFormatter<T>{t, sep, prefix, suffix};
}

} // namespace verible
Expand Down
6 changes: 4 additions & 2 deletions common/strings/display_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ typedef std::vector<int> 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<IntVector> AngleBracketFormatter(
const IntVector& t) {
return SequenceFormatter(t, " | ", "< ", " >");
};
}

TEST(SequenceFormatterTest, AngleBracketVectorNotation) {
const IntVector v{5, 6, 7, 8};
Expand Down
5 changes: 5 additions & 0 deletions common/text/tree_context_visitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ void TreeContextPathVisitor::Visit(const SyntaxTreeNode& node) {
}
}

SequenceStreamFormatter<SyntaxTreePath> TreePathFormatter(
const SyntaxTreePath& path) {
return SequenceFormatter(path, ",", "[", "]");
}

SyntaxTreePath NextSiblingPath(const SyntaxTreePath& path) {
CHECK(!path.empty());
auto next = path;
Expand Down
6 changes: 3 additions & 3 deletions common/text/tree_context_visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<SyntaxTreePath> TreePathFormatter(
const SyntaxTreePath& path);

} // namespace verible

Expand Down
1 change: 1 addition & 0 deletions verilog/formatting/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
56 changes: 16 additions & 40 deletions verilog/formatting/align.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand All @@ -59,6 +62,7 @@ using verible::SyntaxTreePath;
using verible::TokenInfo;
using verible::TokenPartitionTree;
using verible::TreeContextPathVisitor;
using verible::TreePathFormatter;
using verible::ValueSaver;

template <class T>
Expand All @@ -82,36 +86,6 @@ static bool IgnorePartition(const TokenPartitionTree& partition) {
return false;
}

template <class T>
struct SequenceStreamPrinter {
const T& sequence;
absl::string_view separator;
absl::string_view begin;
absl::string_view end;
};

template <class T>
std::ostream& operator<<(std::ostream& stream,
const SequenceStreamPrinter<T>& t) {
return stream << t.begin
<< absl::StrJoin(t.sequence.begin(), t.sequence.end(),
t.separator, absl::StreamFormatter())
<< t.end;
}

template <class T>
SequenceStreamPrinter<T> SequencePrinter(const T& t,
absl::string_view sep = ", ",
absl::string_view begin = "",
absl::string_view end = "") {
return SequenceStreamPrinter<T>{t, sep, begin, end};
}

static SequenceStreamPrinter<SyntaxTreePath> PathPrinter(
const SyntaxTreePath& path) {
return SequencePrinter(path, ",", "[", "]");
}

using TokenPartitionIterator = std::vector<TokenPartitionTree>::iterator;
using TokenPartitionRange =
verible::container_iterator_range<TokenPartitionIterator>;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -460,9 +434,9 @@ class PortDeclarationColumnSchemaScanner : public ColumnSchemaScanner {
}
};

static SequenceStreamPrinter<AlignmentRow> MatrixRowPrinter(
static SequenceStreamFormatter<AlignmentRow> MatrixRowFormatter(
const AlignmentRow& row) {
return SequencePrinter(row, " | ", "<", ">");
return SequenceFormatter(row, " | ", "<", ">");
}

// Translate a sparse set of columns into a fully-populated matrix row.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 << '['
Expand All @@ -548,7 +524,7 @@ static void ComputeCellWidths(AlignmentMatrix* matrix) {
}
}
VLOG(2) << "end of " << __FUNCTION__ << ", cell sizes:\n"
<< MatrixCellSizePrinter{*matrix};
<< MatrixCellSizeFormatter{*matrix};
}

typedef std::vector<AlignedColumnConfiguration> AlignedFormattingColumnSchema;
Expand Down

0 comments on commit 629d890

Please sign in to comment.