[committed,066/103] gccrs: ast: transform helper methods to visits and add methods to simplify repeated patterns

Message ID 20230221120230.596966-67-arthur.cohen@embecosm.com
State Unresolved
Headers
Series [committed,001/103] gccrs: Fix missing dead code analysis ICE on local enum definition |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Arthur Cohen Feb. 21, 2023, 12:01 p.m. UTC
  From: Jakub Dupak <dev@jakubdupak.com>

gcc/rust/ChangeLog:

	* ast/rust-ast-dump.cc (Dump::go): Use new API.
	(Dump::format_function_param): Refactor.
	(Dump::visit_items_joined_by_separator): New function.
	(Dump::emit_attrib): Refactor.
	(Dump::visit_as_line): New function.
	(Dump::visit_items_as_lines): Likewise.
	(Dump::visit_items_as_block): Likewise.
	(Dump::visit): Use new API.
	(Dump::emit_visibility): Likewise.
	(Dump::emit_indented_string): Likewise.
	(Dump::emit_generic_params): Likewise.
	(Dump::format_tuple_field): Likewise.
	(Dump::format_struct_field): Likewise.
	(Dump::format_function_common): Likewise.
	(Dump::visit_function_common): Likewise.
	* ast/rust-ast-dump.h: Declare new functions and add documentation.

Signed-off-by: Jakub Dupak <dev@jakubdupak.com>
---
 gcc/rust/ast/rust-ast-dump.cc | 559 ++++++++++++----------------------
 gcc/rust/ast/rust-ast-dump.h  |  63 ++--
 2 files changed, 235 insertions(+), 387 deletions(-)
  

Patch

diff --git a/gcc/rust/ast/rust-ast-dump.cc b/gcc/rust/ast/rust-ast-dump.cc
index 9771f432ea0..191e328b134 100644
--- a/gcc/rust/ast/rust-ast-dump.cc
+++ b/gcc/rust/ast/rust-ast-dump.cc
@@ -48,12 +48,7 @@  Dump::Dump (std::ostream &stream) : stream (stream), indentation (Indent ()) {}
 void
 Dump::go (AST::Crate &crate)
 {
-  for (auto &item : crate.items)
-    {
-      stream << indentation;
-      item->accept_vis (*this);
-      stream << '\n';
-    }
+  visit_items_as_lines (crate.items, "");
 }
 
 void
@@ -69,28 +64,75 @@  Dump::visit (std::unique_ptr<T> &node)
   node->accept_vis (*this);
 }
 
+template <typename T>
 void
-Dump::format_function_param (FunctionParam &param)
+Dump::visit_items_joined_by_separator (T &collection,
+				       const std::string &separator,
+				       size_t start_offset, size_t end_offset)
 {
-  param.get_pattern ()->accept_vis (*this);
-  stream << ": ";
-  param.get_type ()->accept_vis (*this);
+  if (collection.size () > start_offset)
+    {
+      visit (collection.at (start_offset));
+      auto size = collection.size () - end_offset;
+      for (size_t i = start_offset + 1; i < size; i++)
+	{
+	  stream << separator;
+	  visit (collection.at (i));
+	}
+    }
 }
 
+template <typename T>
 void
-Dump::emit_attrib (const Attribute &attrib)
+Dump::visit_as_line (T &item, const std::string &trailing)
 {
-  stream << "#[";
+  stream << indentation;
+  visit (item);
+  stream << trailing << '\n';
+}
 
-  for (size_t i = 0; i < attrib.get_path ().get_segments ().size (); i++)
+template <typename T>
+void
+Dump::visit_items_as_lines (T &collection, const std::string &trailing)
+{
+  for (auto &item : collection)
+    visit_as_line (item, trailing);
+}
+
+template <typename T>
+void
+Dump::visit_items_as_block (T &collection, const std::string &line_trailing,
+			    char left_brace, char right_brace)
+{
+  if (collection.empty ())
     {
-      const auto &seg = attrib.get_path ().get_segments ().at (i);
-      bool has_next = (i + 1) < attrib.get_path ().get_segments ().size ();
+      stream << left_brace << right_brace << '\n';
+    }
+  else
+    {
+      stream << left_brace << '\n';
+
+      indentation.increment ();
+      visit_items_as_lines (collection, line_trailing);
+      indentation.decrement ();
 
-      stream << seg.get_segment_name ();
-      if (has_next)
-	stream << "::";
+      stream << indentation << right_brace << '\n';
     }
+}
+
+void
+Dump::visit (FunctionParam &param)
+{
+  visit (param.get_pattern ());
+  stream << ": ";
+  visit (param.get_type ());
+}
+
+void
+Dump::visit (const Attribute &attrib)
+{
+  stream << "#[";
+  visit_items_joined_by_separator (attrib.get_path ().get_segments (), "::");
 
   if (attrib.has_attr_input ())
     {
@@ -116,7 +158,13 @@  Dump::emit_attrib (const Attribute &attrib)
 }
 
 void
-Dump::emit_visibility (const Visibility &vis)
+Dump::visit (const SimplePathSegment &segment)
+{
+  stream << segment.get_segment_name ();
+}
+
+void
+Dump::visit (const Visibility &vis)
 {
   switch (vis.get_vis_type ())
     {
@@ -140,44 +188,36 @@  Dump::emit_visibility (const Visibility &vis)
     }
 }
 
-std::ostream &
-Dump::emit_indented_string (const std::string &value,
-			    const std::string &comment)
+void
+Dump::visit (NamedFunctionParam &param)
 {
-  return stream << indentation << value << comment;
+  stream << param.get_name () << ": ";
+  visit (param.get_type ());
 }
 
 void
-Dump::emit_generic_params (std::vector<std::unique_ptr<GenericParam>> &params)
+Dump::visit (std::vector<std::unique_ptr<GenericParam>> &params)
 {
   stream << "<";
-  for (size_t i = 0; i < params.size (); i++)
-    {
-      auto &param = params.at (i);
-      param->accept_vis (*this);
-
-      bool has_next = (i + 1) < params.size ();
-      if (has_next)
-	stream << ", ";
-    }
+  visit_items_joined_by_separator (params, ", ");
   stream << ">";
 }
 
 void
-Dump::format_tuple_field (TupleField &field)
+Dump::visit (TupleField &field)
 {
   // TODO: do we need to emit outer attrs here?
-  emit_visibility (field.get_visibility ());
-  field.get_field_type ()->accept_vis (*this);
+  visit (field.get_visibility ());
+  visit (field.get_field_type ());
 }
 
 void
-Dump::format_struct_field (StructField &field)
+Dump::visit (StructField &field)
 {
   // TODO: do we need to emit outer attrs here?
-  emit_visibility (field.get_visibility ());
+  visit (field.get_visibility ());
   stream << field.get_field_name () << ": ";
-  field.get_field_type ()->accept_vis (*this);
+  visit (field.get_field_type ());
 }
 
 void
@@ -189,16 +229,11 @@  Dump::visit (Token &tok)
 void
 Dump::visit (DelimTokenTree &delim_tok_tree)
 {
-  auto tokens = delim_tok_tree.to_token_stream ();
-
   indentation.increment ();
   stream << '\n' << indentation;
 
-  for (const auto &tok : tokens)
-    {
-      stream << ' ';
-      tok->accept_vis (*this);
-    }
+  auto tokens = delim_tok_tree.to_token_stream ();
+  visit_items_joined_by_separator (tokens, " ");
 
   indentation.decrement ();
   stream << '\n' << indentation;
@@ -289,20 +324,20 @@  Dump::visit (BorrowExpr &expr)
   if (expr.get_is_mut ())
     stream << "mut ";
 
-  expr.get_borrowed_expr ()->accept_vis (*this);
+  visit (expr.get_borrowed_expr ());
 }
 
 void
 Dump::visit (DereferenceExpr &expr)
 {
   stream << '*';
-  expr.get_dereferenced_expr ()->accept_vis (*this);
+  visit (expr.get_dereferenced_expr ());
 }
 
 void
 Dump::visit (ErrorPropagationExpr &expr)
 {
-  expr.get_propagating_expr ()->accept_vis (*this);
+  visit (expr.get_propagating_expr ());
   stream << '?';
 }
 
@@ -318,7 +353,7 @@  Dump::visit (NegationExpr &expr)
       stream << '!';
       break;
     }
-  expr.get_negated_expr ()->accept_vis (*this);
+  visit (expr.get_negated_expr ());
 }
 
 void
@@ -368,9 +403,9 @@  Dump::visit (ArithmeticOrLogicalExpr &expr)
       break;
     }
 
-  expr.get_left_expr ()->accept_vis (*this);
+  visit (expr.get_left_expr ());
   stream << " " << op << " ";
-  expr.get_right_expr ()->accept_vis (*this);
+  visit (expr.get_right_expr ());
 }
 
 void
@@ -403,9 +438,9 @@  Dump::visit (ComparisonExpr &expr)
       break;
     }
 
-  expr.get_left_expr ()->accept_vis (*this);
+  visit (expr.get_left_expr ());
   stream << " " << op << " ";
-  expr.get_right_expr ()->accept_vis (*this);
+  visit (expr.get_right_expr ());
 }
 
 void
@@ -422,17 +457,17 @@  Dump::visit (LazyBooleanExpr &expr)
       break;
     }
 
-  expr.get_left_expr ()->accept_vis (*this);
+  visit (expr.get_left_expr ());
   stream << " " << op << " ";
-  expr.get_right_expr ()->accept_vis (*this);
+  visit (expr.get_right_expr ());
 }
 
 void
 Dump::visit (TypeCastExpr &expr)
 {
-  expr.get_casted_expr ()->accept_vis (*this);
+  visit (expr.get_casted_expr ());
   stream << " as ";
-  expr.get_type_to_cast_to ()->accept_vis (*this);
+  visit (expr.get_type_to_cast_to ());
 }
 
 void
@@ -490,56 +525,47 @@  Dump::visit (CompoundAssignmentExpr &expr)
       break;
     }
 
-  expr.get_left_expr ()->accept_vis (*this);
+  visit (expr.get_left_expr ());
   stream << " " << op << "= ";
-  expr.get_right_expr ()->accept_vis (*this);
+  visit (expr.get_right_expr ());
 }
 
 void
 Dump::visit (GroupedExpr &expr)
 {
   stream << '(';
-  expr.get_expr_in_parens ()->accept_vis (*this);
+  visit (expr.get_expr_in_parens ());
   stream << ')';
 }
 
 void
 Dump::visit (ArrayElemsValues &elems)
 {
-  auto &vals = elems.get_values ();
-  if (vals.size () >= 1)
-    {
-      vals[0]->accept_vis (*this);
-      for (size_t i = 1; i < vals.size (); i++)
-	{
-	  stream << ", ";
-	  vals[i]->accept_vis (*this);
-	}
-    }
+  visit_items_joined_by_separator (elems.get_values (), ", ");
 }
 
 void
 Dump::visit (ArrayElemsCopied &elems)
 {
-  elems.get_elem_to_copy ()->accept_vis (*this);
+  visit (elems.get_elem_to_copy ());
   stream << "; ";
-  elems.get_num_copies ()->accept_vis (*this);
+  visit (elems.get_num_copies ());
 }
 
 void
 Dump::visit (ArrayExpr &expr)
 {
   stream << '[';
-  expr.get_array_elems ()->accept_vis (*this);
+  visit (expr.get_array_elems ());
   stream << ']';
 }
 
 void
 Dump::visit (ArrayIndexExpr &expr)
 {
-  expr.get_array_expr ()->accept_vis (*this);
+  visit (expr.get_array_expr ());
   stream << '[';
-  expr.get_index_expr ()->accept_vis (*this);
+  visit (expr.get_index_expr ());
   stream << ']';
 }
 
@@ -578,21 +604,15 @@  Dump::visit (StructExprStructBase &expr)
 void
 Dump::visit (CallExpr &expr)
 {
-  expr.get_function_expr ()->accept_vis (*this);
-  stream << '(';
+  visit (expr.get_function_expr ());
 
+  stream << '(' << '\n';
   indentation.increment ();
 
-  for (auto &arg : expr.get_params ())
-    {
-      stream << '\n' << indentation;
-      arg->accept_vis (*this);
-      stream << ',';
-    }
+  visit_items_as_lines (expr.get_params (), ",");
 
   indentation.decrement ();
-
-  stream << '\n' << indentation << ')';
+  stream << indentation << ')';
 }
 
 void
@@ -613,19 +633,10 @@  Dump::visit (BlockExpr &expr)
   stream << "{\n";
   indentation.increment ();
 
-  for (auto &stmt : expr.get_statements ())
-    {
-      stream << indentation;
-      stmt->accept_vis (*this);
-      stream << "; /* stmt */\n";
-    }
+  visit_items_as_lines (expr.get_statements (), "; /* stmt */");
 
   if (expr.has_tail_expr ())
-    {
-      stream << indentation;
-      expr.get_tail_expr ()->accept_vis (*this);
-      stream << " /* tail expr */\n";
-    }
+    visit_as_line (expr.get_tail_expr (), " /* tail expr */\n");
 
   indentation.decrement ();
   stream << indentation << "}\n";
@@ -646,15 +657,15 @@  Dump::visit (BreakExpr &expr)
 void
 Dump::visit (RangeFromToExpr &expr)
 {
-  expr.get_from_expr ()->accept_vis (*this);
+  visit (expr.get_from_expr ());
   stream << "..";
-  expr.get_to_expr ()->accept_vis (*this);
+  visit (expr.get_to_expr ());
 }
 
 void
 Dump::visit (RangeFromExpr &expr)
 {
-  expr.get_from_expr ()->accept_vis (*this);
+  visit (expr.get_from_expr ());
   stream << "..";
 }
 
@@ -662,7 +673,7 @@  void
 Dump::visit (RangeToExpr &expr)
 {
   stream << "..";
-  expr.get_to_expr ()->accept_vis (*this);
+  visit (expr.get_to_expr ());
 }
 
 void
@@ -674,16 +685,16 @@  Dump::visit (RangeFullExpr &expr)
 void
 Dump::visit (RangeFromToInclExpr &expr)
 {
-  expr.get_from_expr ()->accept_vis (*this);
+  visit (expr.get_from_expr ());
   stream << "..=";
-  expr.get_to_expr ()->accept_vis (*this);
+  visit (expr.get_to_expr ());
 }
 
 void
 Dump::visit (RangeToInclExpr &expr)
 {
   stream << "..=";
-  expr.get_to_expr ()->accept_vis (*this);
+  visit (expr.get_to_expr ());
 }
 
 void
@@ -714,32 +725,32 @@  void
 Dump::visit (IfExpr &expr)
 {
   stream << "if ";
-  expr.vis_if_condition (*this);
+  visit (expr.get_condition_expr ());
   stream << " ";
-  expr.vis_if_block (*this);
+  visit (expr.get_if_block ());
 }
 
 void
 Dump::visit (IfExprConseqElse &expr)
 {
   stream << "if ";
-  expr.vis_if_condition (*this);
+  visit (expr.get_condition_expr ());
   stream << " ";
-  expr.vis_if_block (*this);
+  visit (expr.get_if_block ());
   stream << indentation << "else ";
-  expr.vis_else_block (*this);
+  visit (expr.get_else_block ());
 }
 
 void
 Dump::visit (IfExprConseqIf &expr)
 {
   stream << "if ";
-  expr.vis_if_condition (*this);
+  visit (expr.get_condition_expr ());
   stream << " ";
-  expr.vis_if_block (*this);
+  visit (expr.get_if_block ());
   stream << indentation << "else ";
   // The "if" part of the "else if" is printed by the next visitor
-  expr.vis_conseq_if_expr (*this);
+  visit (expr.get_conseq_if_expr ());
 }
 
 void
@@ -782,7 +793,7 @@  Dump::visit (TypeParam &param)
   if (param.has_type ())
     {
       stream << " = ";
-      param.get_type ()->accept_vis (*this);
+      visit (param.get_type ());
     }
 }
 
@@ -799,25 +810,18 @@  Dump::visit (Method &method)
 {
   // FIXME: Do we really need to dump the indentation here?
   stream << indentation;
-  emit_visibility (method.get_visibility ());
+  visit (method.get_visibility ());
   stream << "fn " << method.get_method_name () << '(';
 
-  auto &self = method.get_self_param ();
-  stream << self.as_string ();
-
-  auto &params = method.get_function_params ();
-  for (auto &param : params)
-    {
-      stream << ", ";
-      format_function_param (param);
-    }
+  stream << method.get_self_param ().as_string () << ", ";
+  visit_items_joined_by_separator (method.get_function_params (), ", ");
 
   stream << ") ";
 
   if (method.has_return_type ())
     {
       stream << "-> ";
-      method.get_return_type ()->accept_vis (*this);
+      visit (method.get_return_type ());
       stream << " ";
     }
 
@@ -825,7 +829,7 @@  Dump::visit (Method &method)
   if (!block)
     stream << ';';
   else
-    block->accept_vis (*this);
+    visit (block);
 
   stream << '\n';
 }
@@ -840,7 +844,7 @@  Dump::visit (Module &module)
   //	  Item*
   //	}
 
-  emit_visibility (module.get_visibility ());
+  visit (module.get_visibility ());
   stream << "mod " << module.get_name ();
 
   if (module.get_kind () == Module::UNLOADED)
@@ -853,19 +857,8 @@  Dump::visit (Module &module)
 
       indentation.increment ();
 
-      for (auto &item : module.get_inner_attrs ())
-	{
-	  stream << indentation;
-	  emit_attrib (item);
-	  stream << '\n';
-	}
-
-      for (auto &item : module.get_items ())
-	{
-	  stream << indentation;
-	  item->accept_vis (*this);
-	  stream << '\n';
-	}
+      visit_items_as_lines (module.get_inner_attrs ());
+      visit_items_as_lines (module.get_items ());
 
       indentation.decrement ();
 
@@ -896,30 +889,20 @@  Dump::visit (UseDeclaration &use_decl)
 void
 Dump::visit (Function &function)
 {
-  emit_visibility (function.get_visibility ());
+  visit (function.get_visibility ());
 
   stream << "fn " << function.get_function_name ();
   if (function.has_generics ())
-    emit_generic_params (function.get_generic_params ());
+    visit (function.get_generic_params ());
 
   stream << '(';
-  auto &params = function.get_function_params ();
-  if (params.size () >= 1)
-    {
-      format_function_param (params[0]);
-      for (size_t i = 1; i < params.size (); i++)
-	{
-	  stream << ", ";
-	  format_function_param (params[i]);
-	}
-    }
-
+  visit_items_joined_by_separator (function.get_function_params ());
   stream << ") ";
 
   if (function.has_return_type ())
     {
       stream << "-> ";
-      function.get_return_type ()->accept_vis (*this);
+      visit (function.get_return_type ());
       stream << " ";
     }
 
@@ -927,7 +910,7 @@  Dump::visit (Function &function)
   if (!block)
     stream << ';';
   else
-    block->accept_vis (*this);
+    visit (block);
 
   stream << '\n';
 }
@@ -941,15 +924,15 @@  Dump::visit (TypeAlias &type_alias)
   // Note: Associated types are handled by `AST::TraitItemType`.
 
   if (type_alias.has_visibility ())
-    emit_visibility (type_alias.get_visibility ());
+    visit (type_alias.get_visibility ());
   stream << "type " << type_alias.get_new_type_name ();
   if (type_alias.has_generics ())
-    emit_generic_params (type_alias.get_generic_params ());
+    visit (type_alias.get_generic_params ());
   if (type_alias.has_where_clause ())
     {
     } // FIXME: WhereClause
   stream << " = ";
-  type_alias.get_type_aliased ()->accept_vis (*this);
+  visit (type_alias.get_type_aliased ());
   stream << ";\n";
 }
 
@@ -958,26 +941,11 @@  Dump::visit (StructStruct &struct_item)
 {
   stream << "struct " << struct_item.get_identifier ();
   if (struct_item.has_generics ())
-    emit_generic_params (struct_item.get_generic_params ());
+    visit (struct_item.get_generic_params ());
 
   // FIXME: where-clause
 
-  stream << " {";
-
-  auto &fields = struct_item.get_fields ();
-
-  indentation.increment ();
-  for (auto &field : fields)
-    {
-      stream << '\n' << indentation;
-      format_struct_field (field);
-      stream << ',';
-    }
-  indentation.decrement ();
-
-  if (fields.size () > 0)
-    stream << '\n' << indentation;
-  stream << "}\n";
+  visit_items_as_block (struct_item.get_fields (), ",");
 }
 
 void
@@ -985,22 +953,12 @@  Dump::visit (TupleStruct &tuple_struct)
 {
   stream << "struct " << tuple_struct.get_identifier ();
   if (tuple_struct.has_generics ())
-    emit_generic_params (tuple_struct.get_generic_params ());
+    visit (tuple_struct.get_generic_params ());
 
   // FIXME: where-clause
 
   stream << '(';
-
-  auto &fields = tuple_struct.get_fields ();
-  if (fields.size () >= 1)
-    {
-      format_tuple_field (fields[0]);
-      for (size_t i = 1; i < fields.size (); i++)
-	{
-	  stream << ", ";
-	  format_tuple_field (fields[i]);
-	}
-    }
+  visit_items_joined_by_separator (tuple_struct.get_fields (), ", ");
   stream << ");\n";
 }
 
@@ -1014,45 +972,22 @@  void
 Dump::visit (EnumItemTuple &item)
 {
   stream << item.get_identifier () << '(';
-  auto &fields = item.get_tuple_fields ();
-  if (fields.size () >= 1)
-    {
-      format_tuple_field (fields[0]);
-      for (size_t i = 1; i < fields.size (); i++)
-	{
-	  stream << ", ";
-	  format_tuple_field (fields[i]);
-	}
-    }
+  visit_items_joined_by_separator (item.get_tuple_fields (), ", ");
   stream << ')';
 }
 
 void
 Dump::visit (EnumItemStruct &item)
 {
-  stream << item.get_identifier () << " {";
-
-  auto &fields = item.get_struct_fields ();
-
-  indentation.increment ();
-  for (auto &field : fields)
-    {
-      stream << '\n' << indentation;
-      format_struct_field (field);
-      stream << ',';
-    }
-  indentation.decrement ();
-
-  if (fields.size () > 0)
-    stream << '\n' << indentation;
-  stream << '}';
+  stream << item.get_identifier ();
+  visit_items_as_block (item.get_struct_fields (), ",");
 }
 
 void
 Dump::visit (EnumItemDiscriminant &item)
 {
   stream << item.get_identifier () << " = ";
-  item.get_expr ()->accept_vis (*this);
+  visit (item.get_expr ());
 }
 
 void
@@ -1060,26 +995,11 @@  Dump::visit (Enum &enum_item)
 {
   stream << "enum " << enum_item.get_identifier ();
   if (enum_item.has_generics ())
-    emit_generic_params (enum_item.get_generic_params ());
+    visit (enum_item.get_generic_params ());
 
   // FIXME: where-clause
 
-  stream << " {";
-  auto &variants = enum_item.get_variants ();
-  if (variants.size () >= 1)
-    {
-      stream << '\n';
-      indentation.increment ();
-      for (auto &var : variants)
-	{
-	  stream << indentation;
-	  var->accept_vis (*this);
-	  stream << ",\n";
-	}
-      indentation.decrement ();
-    }
-
-  stream << "}\n";
+  visit_items_as_block (enum_item.get_variants (), ",");
 }
 
 void
@@ -1087,21 +1007,11 @@  Dump::visit (Union &union_item)
 {
   stream << "union " << union_item.get_identifier ();
   if (union_item.has_generics ())
-    emit_generic_params (union_item.get_generic_params ());
+    visit (union_item.get_generic_params ());
 
   // FIXME: where-clause
 
-  stream << " {";
-  indentation.increment ();
-  for (auto &field : union_item.get_variants ())
-    {
-      stream << '\n' << indentation;
-      format_struct_field (field);
-      stream << ',';
-    }
-  indentation.decrement ();
-
-  stream << '\n' << indentation << "}\n";
+  visit_items_as_block (union_item.get_variants (), ",");
 }
 
 void
@@ -1113,14 +1023,14 @@  Dump::visit (StaticItem &static_item)
 {}
 
 void
-Dump::format_function_common (std::unique_ptr<Type> &return_type,
-			      std::unique_ptr<BlockExpr> &block)
+Dump::visit_function_common (std::unique_ptr<Type> &return_type,
+			     std::unique_ptr<BlockExpr> &block)
 {
   // FIXME: This should format the `<vis> fn <name> ( [args] )` as well
   if (return_type)
     {
       stream << "-> ";
-      return_type->accept_vis (*this);
+      visit (return_type);
     }
 
   if (block)
@@ -1128,7 +1038,7 @@  Dump::format_function_common (std::unique_ptr<Type> &return_type,
       if (return_type)
 	{
 	  stream << ' ';
-	  block->accept_vis (*this);
+	  visit (block);
 	}
     }
   else
@@ -1141,16 +1051,11 @@  Dump::visit (TraitItemFunc &item)
   auto func = item.get_trait_function_decl ();
   stream << indentation << "fn " << func.get_identifier () << '(';
 
-  auto &params = func.get_function_params ();
-  for (auto &param : params)
-    {
-      stream << ", ";
-      format_function_param (param);
-    }
+  visit_items_joined_by_separator (func.get_function_params ());
 
   stream << ") ";
 
-  format_function_common (func.get_return_type (), item.get_definition ());
+  visit_function_common (func.get_return_type (), item.get_definition ());
 }
 
 void
@@ -1165,26 +1070,20 @@  Dump::visit (TraitItemMethod &item)
   // emit_visibility (method.get_visibility ());
   stream << "fn " << method.get_identifier () << '(';
 
-  auto &self = method.get_self_param ();
-  stream << self.as_string ();
+  stream << method.get_self_param ().as_string () << ", ";
 
-  auto &params = method.get_function_params ();
-  for (auto &param : params)
-    {
-      stream << ", ";
-      format_function_param (param);
-    }
+  visit_items_joined_by_separator (method.get_function_params (), ", ");
 
   stream << ") ";
 
-  format_function_common (method.get_return_type (), item.get_definition ());
+  visit_function_common (method.get_return_type (), item.get_definition ());
 }
 
 void
 Dump::visit (TraitItemConst &item)
 {
   stream << indentation << "const " << item.get_identifier () << ": ";
-  item.get_type ()->accept_vis (*this);
+  visit (item.get_type ());
   stream << ";\n";
 }
 
@@ -1199,40 +1098,24 @@  Dump::visit (Trait &trait)
 {
   for (const auto &attr : trait.get_outer_attrs ())
     {
-      emit_attrib (attr);
+      visit (attr);
       stream << "\n" << indentation;
     }
 
-  emit_visibility (trait.get_visibility ());
+  visit (trait.get_visibility ());
 
   stream << "trait " << trait.get_identifier ();
 
-  // Traits actually have an implicit Self thrown at the start so we must expect
-  // the number of generic params to be > 1
+  // Traits actually have an implicit Self thrown at the start, so we must
+  // expect the number of generic params to be > 1
   if (trait.get_generic_params ().size () > 1)
     {
       stream << "<";
-      for (size_t i = 1; i < trait.get_generic_params ().size (); i++)
-	{
-	  auto &param = trait.get_generic_params ().at (i);
-	  param->accept_vis (*this);
-
-	  bool has_next = (i + 1) < trait.get_generic_params ().size ();
-	  if (has_next)
-	    stream << ", ";
-	}
+      visit_items_joined_by_separator (trait.get_generic_params (), ", ", 1);
       stream << ">";
     }
 
-  stream << " {\n";
-
-  indentation.increment ();
-
-  for (auto &item : trait.get_trait_items ())
-    item->accept_vis (*this);
-
-  indentation.decrement ();
-  stream << "\n}\n";
+  visit_items_as_block (trait.get_trait_items (), "");
 }
 
 void
@@ -1242,28 +1125,21 @@  Dump::visit (InherentImpl &impl)
 
   // FIXME: Handle generics
 
-  impl.get_type ()->accept_vis (*this);
+  visit (impl.get_type ());
 
   // FIXME: Handle where-clause
   // FIXME: Handle inner attributes
 
-  stream << " {\n";
-  indentation.increment ();
-
-  for (auto &item : impl.get_impl_items ())
-    item->accept_vis (*this);
-
-  indentation.decrement ();
-  stream << "\n}\n";
+  visit_items_as_block (impl.get_impl_items (), "");
 }
 
 void
 Dump::visit (TraitImpl &impl)
 {
   stream << "impl ";
-  impl.get_trait_path ().accept_vis (*this);
+  visit (impl.get_trait_path ());
   stream << " for ";
-  impl.get_type ()->accept_vis (*this);
+  visit (impl.get_type ());
   stream << " {\n";
 
   indentation.increment ();
@@ -1271,7 +1147,7 @@  Dump::visit (TraitImpl &impl)
   for (auto &item : impl.get_impl_items ())
     {
       stream << indentation;
-      item->accept_vis (*this);
+      visit (item);
     }
 
   indentation.decrement ();
@@ -1285,27 +1161,17 @@  Dump::visit (ExternalStaticItem &item)
 void
 Dump::visit (ExternalFunctionItem &function)
 {
-  emit_visibility (function.get_visibility ());
+  visit (function.get_visibility ());
 
   stream << "fn " << function.get_identifier () << '(';
 
-  for (size_t i = 0; i < function.get_function_params ().size (); i++)
-    {
-      auto &param = function.get_function_params ().at (i);
-      bool has_next = (i + 1) < function.get_function_params ().size ();
-
-      stream << param.get_name () << ": ";
-      param.get_type ()->accept_vis (*this);
-
-      if (has_next)
-	stream << ", ";
-    }
+  visit_items_joined_by_separator (function.get_function_params ());
 
   stream << ')';
   if (function.has_return_type ())
     {
       stream << "-> ";
-      function.get_return_type ()->accept_vis (*this);
+      visit (function.get_return_type ());
     }
 }
 
@@ -1317,18 +1183,7 @@  Dump::visit (ExternBlock &block)
   if (block.has_abi ())
     stream << "\"" << block.get_abi () << "\" ";
 
-  stream << "{\n";
-  indentation.increment ();
-
-  for (auto &item : block.get_extern_items ())
-    {
-      stream << indentation;
-      item->accept_vis (*this);
-      stream << ";\n";
-    }
-
-  indentation.decrement ();
-  stream << "\n" << indentation << "}\n";
+  visit_items_as_block (block.get_extern_items (), ";");
 }
 
 static std::pair<char, char>
@@ -1368,11 +1223,7 @@  Dump::visit (MacroMatchRepetition &repetition)
 {
   stream << "$(";
 
-  for (auto &match : repetition.get_matches ())
-    {
-      match->accept_vis (*this);
-      stream << ' ';
-    }
+  visit_items_joined_by_separator (repetition.get_matches (), " ");
 
   auto op_char = '\0';
   switch (repetition.get_op ())
@@ -1405,41 +1256,29 @@  Dump::visit (MacroMatcher &matcher)
 
   stream << delimiters.first;
 
-  for (auto &match : matcher.get_matches ())
-    {
-      match->accept_vis (*this);
-      stream << ' ';
-    }
+  visit_items_joined_by_separator (matcher.get_matches (), " ");
 
   stream << delimiters.second;
 }
 
+void
+Dump::visit (MacroRule &rule)
+{
+  visit (rule.get_matcher ());
+  stream << " => ";
+  visit (rule.get_transcriber ().get_token_tree ());
+  stream << ";";
+}
+
 void
 Dump::visit (MacroRulesDefinition &rules_def)
 {
   for (auto &outer_attr : rules_def.get_outer_attrs ())
-    emit_attrib (outer_attr);
-
-  stream << "macro_rules! " << rules_def.get_rule_name () << " {\n";
-
-  indentation.increment ();
+    visit (outer_attr);
 
-  for (auto &rule : rules_def.get_rules ())
-    {
-      stream << indentation;
-
-      rule.get_matcher ().accept_vis (*this);
-
-      stream << " => ";
-
-      rule.get_transcriber ().get_token_tree ().accept_vis (*this);
-
-      stream << ";\n";
-    }
-
-  indentation.decrement ();
+  stream << "macro_rules! " << rules_def.get_rule_name ();
 
-  stream << "}\n";
+  visit_items_as_block (rules_def.get_rules (), ";");
 }
 
 void
@@ -1572,31 +1411,31 @@  Dump::visit (LetStmt &stmt)
   stream << "let ";
   auto &pattern = stmt.get_pattern ();
   if (pattern)
-    pattern->accept_vis (*this);
+    visit (pattern);
 
   if (stmt.has_type ())
     {
       stream << ": ";
-      stmt.get_type ()->accept_vis (*this);
+      visit (stmt.get_type ());
     }
 
   if (stmt.has_init_expr ())
     {
       stream << " = ";
-      stmt.get_init_expr ()->accept_vis (*this);
+      visit (stmt.get_init_expr ());
     }
 }
 
 void
 Dump::visit (ExprStmtWithoutBlock &stmt)
 {
-  stmt.get_expr ()->accept_vis (*this);
+  visit (stmt.get_expr ());
 }
 
 void
 Dump::visit (ExprStmtWithBlock &stmt)
 {
-  stmt.get_expr ()->accept_vis (*this);
+  visit (stmt.get_expr ());
 }
 
 // rust-type.h
@@ -1639,19 +1478,19 @@  Dump::visit (RawPointerType &type)
 void
 Dump::visit (ReferenceType &type)
 {
-  type.get_type_referenced ()->accept_vis (*this);
+  visit (type.get_type_referenced ());
 }
 
 void
 Dump::visit (ArrayType &type)
 {
-  type.get_elem_type ()->accept_vis (*this);
+  visit (type.get_elem_type ());
 }
 
 void
 Dump::visit (SliceType &type)
 {
-  type.get_elem_type ()->accept_vis (*this);
+  visit (type.get_elem_type ());
 }
 
 void
diff --git a/gcc/rust/ast/rust-ast-dump.h b/gcc/rust/ast/rust-ast-dump.h
index 7cd922e0ac9..7a8058b195e 100644
--- a/gcc/rust/ast/rust-ast-dump.h
+++ b/gcc/rust/ast/rust-ast-dump.h
@@ -81,46 +81,55 @@  private:
   template <typename T> void visit (std::unique_ptr<T> &node);
 
   /**
-   * Format together common items of functions: Parameters, return type, block
+   * Visit all items in given collection, placing the separator in between but
+   * not at the end.
+   * Start and end offset allow to visit only a "slice" from the collection.
    */
-  void format_function_common (std::unique_ptr<Type> &return_type,
-			       std::unique_ptr<BlockExpr> &block);
+  template <typename T>
+  void visit_items_joined_by_separator (T &collection,
+					const std::string &separator = "",
+					size_t start_offset = 0,
+					size_t end_offset = 0);
 
   /**
-   * Format a function's definition parameter
+   * Visit item placing indentation before and trailing string + end of line
+   * after.
    */
-  void format_function_param (FunctionParam &param);
+  template <typename T>
+  void visit_as_line (T &item, const std::string &trailing = "");
 
   /**
-   * Emit an attribute
+   * Visit each item in a collection "as line".
+   *
+   * @see visit_as_line
    */
-  void emit_attrib (const Attribute &attrib);
+  template <typename T>
+  void visit_items_as_lines (T &collection, const std::string &trailing = "");
 
   /**
-   * Emit an item's visibility
+   * Visit each item in collection as lines inside a block delimited by braces
+   * with increased indentation. Also includes special handling for empty
+   * collection to print only the delimiters with no new line inside.
    */
-  void emit_visibility (const Visibility &vis);
+  template <typename T>
+  void visit_items_as_block (T &collection, const std::string &line_trailing,
+			     char left_brace = '{', char right_brace = '}');
 
   /**
-   * Emit an indented string with an optional extra comment
+   * Visit common items of functions: Parameters, return type, block
    */
-  std::ostream &emit_indented_string (const std::string &value,
-				      const std::string &comment = "");
-
-  /**
-   * Emit formatted string for generic parameters
-   */
-  void emit_generic_params (std::vector<std::unique_ptr<GenericParam>> &params);
-
-  /**
-   * Format a single field of a tuple
-   */
-  void format_tuple_field (TupleField &field);
-
-  /**
-   * Format a single field of a struct
-   */
-  void format_struct_field (StructField &field);
+  void visit_function_common (std::unique_ptr<Type> &return_type,
+			      std::unique_ptr<BlockExpr> &block);
+
+  void visit (FunctionParam &param);
+  void visit (const Attribute &attrib);
+  void visit (const Visibility &vis);
+  void visit (std::vector<std::unique_ptr<GenericParam>> &params);
+  void visit (TupleField &field);
+  void visit (StructField &field);
+  void visit (const SimplePathSegment &segment);
+  void visit (NamedFunctionParam &param);
+  void visit (MacroRule &rule);
 
   // rust-ast.h
   void visit (Token &tok);