public inbox for gcc-rust@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Remove lambda iterators in various HIR classes
@ 2021-10-08 17:45 David Faust
  2021-10-08 17:49 ` David Faust
  0 siblings, 1 reply; 3+ messages in thread
From: David Faust @ 2021-10-08 17:45 UTC (permalink / raw)
  To: gcc-rust

This patch removes the lambda iterators used in various HIR objects.
These iterators make interacting with the IR for static analysis more
difficult. Instead, get_X () helpers are added for accessing elements,
and uses of the iterators replaced with for loops.

The following objects are adjusted in this patch:
- HIR::TupleExpr
- HIR::StructExprField
- HIR::StructStruct
- HIR::TupleStruct

Fixes: #704, #705, #706, #707
---
 gcc/rust/backend/rust-compile-expr.h          | 20 ++++-----
 gcc/rust/hir/tree/rust-hir-expr.h             | 32 +------------
 gcc/rust/hir/tree/rust-hir-item.h             | 18 +-------
 gcc/rust/lint/rust-lint-marklive.h            | 24 +++++-----
 gcc/rust/lint/rust-lint-scan-deadcode.h       | 20 ++++-----
 gcc/rust/typecheck/rust-hir-type-check-expr.h |  9 ++--
 gcc/rust/typecheck/rust-hir-type-check-stmt.h | 44 +++++++++---------
 .../typecheck/rust-hir-type-check-toplevel.h  | 45 ++++++++++---------
 gcc/rust/typecheck/rust-hir-type-check.cc     | 33 +++++++-------
 gcc/rust/typecheck/rust-tycheck-dump.h        | 10 ++---
 10 files changed, 107 insertions(+), 148 deletions(-)

diff --git a/gcc/rust/backend/rust-compile-expr.h b/gcc/rust/backend/rust-compile-expr.h
index eb245dce5be..7c4046680e9 100644
--- a/gcc/rust/backend/rust-compile-expr.h
+++ b/gcc/rust/backend/rust-compile-expr.h
@@ -412,11 +412,11 @@ public:
 
   void visit (HIR::ArrayElemsValues &elems) override
   {
-    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
-      Bexpression *translated_expr = CompileExpr::Compile (e, ctx);
-      constructor.push_back (translated_expr);
-      return true;
-    });
+    for (auto &elem : elems.get_values ())
+      {
+	Bexpression *translated_expr = CompileExpr::Compile (elem.get (), ctx);
+	constructor.push_back (translated_expr);
+      }
   }
 
   void visit (HIR::ArrayElemsCopied &elems) override
@@ -646,11 +646,11 @@ public:
     // this assumes all fields are in order from type resolution and if a base
     // struct was specified those fields are filed via accesors
     std::vector<Bexpression *> vals;
-    struct_expr.iterate ([&] (HIR::StructExprField *field) mutable -> bool {
-      Bexpression *expr = CompileStructExprField::Compile (field, ctx);
-      vals.push_back (expr);
-      return true;
-    });
+    for (auto &field : struct_expr.get_fields ())
+      {
+	Bexpression *expr = CompileStructExprField::Compile (field.get (), ctx);
+	vals.push_back (expr);
+      }
 
     translated
       = ctx->get_backend ()->constructor_expression (type, vals,
diff --git a/gcc/rust/hir/tree/rust-hir-expr.h b/gcc/rust/hir/tree/rust-hir-expr.h
index 05bc1f9f055..d9958a153be 100644
--- a/gcc/rust/hir/tree/rust-hir-expr.h
+++ b/gcc/rust/hir/tree/rust-hir-expr.h
@@ -796,14 +796,7 @@ public:
 
   size_t get_num_elements () const { return values.size (); }
 
-  void iterate (std::function<bool (Expr *)> cb)
-  {
-    for (auto it = values.begin (); it != values.end (); it++)
-      {
-	if (!cb ((*it).get ()))
-	  return;
-      }
-  }
+  std::vector<std::unique_ptr<Expr>>& get_values () { return values; }
 
 protected:
   ArrayElemsValues *clone_array_elems_impl () const override
@@ -1070,15 +1063,6 @@ public:
 
   bool is_unit () const { return tuple_elems.size () == 0; }
 
-  void iterate (std::function<bool (Expr *)> cb)
-  {
-    for (auto &tuple_elem : tuple_elems)
-      {
-	if (!cb (tuple_elem.get ()))
-	  return;
-      }
-  }
-
 protected:
   /* Use covariance to implement clone function as returning this object rather
    * than base */
@@ -1491,15 +1475,6 @@ public:
 
   void accept_vis (HIRVisitor &vis) override;
 
-  void iterate (std::function<bool (StructExprField *)> cb)
-  {
-    for (auto &field : fields)
-      {
-	if (!cb (field.get ()))
-	  return;
-      }
-  }
-
   std::vector<std::unique_ptr<StructExprField> > &get_fields ()
   {
     return fields;
@@ -1510,11 +1485,6 @@ public:
     return fields;
   };
 
-  std::vector<std::unique_ptr<StructExprField> > get_fields_as_owner ()
-  {
-    return std::move (fields);
-  };
-
   void set_fields_as_owner (
     std::vector<std::unique_ptr<StructExprField> > new_fields)
   {
diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
index 8cd7a01b2e8..f5119c3dc77 100644
--- a/gcc/rust/hir/tree/rust-hir-item.h
+++ b/gcc/rust/hir/tree/rust-hir-item.h
@@ -1492,14 +1492,7 @@ public:
 
   void accept_vis (HIRVisitor &vis) override;
 
-  void iterate (std::function<bool (StructField &)> cb)
-  {
-    for (auto &field : fields)
-      {
-	if (!cb (field))
-	  return;
-      }
-  }
+  std::vector<StructField>& get_fields() { return fields; }
 
 protected:
   /* Use covariance to implement clone function as returning this object
@@ -1610,15 +1603,6 @@ public:
   std::vector<TupleField> &get_fields () { return fields; }
   const std::vector<TupleField> &get_fields () const { return fields; }
 
-  void iterate (std::function<bool (TupleField &)> cb)
-  {
-    for (auto &field : fields)
-      {
-	if (!cb (field))
-	  return;
-      }
-  }
-
 protected:
   /* Use covariance to implement clone function as returning this object
    * rather than base */
diff --git a/gcc/rust/lint/rust-lint-marklive.h b/gcc/rust/lint/rust-lint-marklive.h
index 062bb96bc0f..cd72ef0243d 100644
--- a/gcc/rust/lint/rust-lint-marklive.h
+++ b/gcc/rust/lint/rust-lint-marklive.h
@@ -81,18 +81,18 @@ public:
 
   void visit (HIR::ArrayElemsValues &expr) override
   {
-    expr.iterate ([&] (HIR::Expr *expr) mutable -> bool {
-      expr->accept_vis (*this);
-      return true;
-    });
+    for (auto &elem : expr.get_values ())
+      {
+	elem->accept_vis (*this);
+      }
   }
 
   void visit (HIR::TupleExpr &expr) override
   {
-    expr.iterate ([&] (HIR::Expr *expr) mutable -> bool {
-      expr->accept_vis (*this);
-      return true;
-    });
+    for (auto &elem : expr.get_tuple_elems ())
+      {
+	elem->accept_vis (*this);
+      }
   }
 
   void visit (HIR::BlockExpr &expr) override
@@ -236,10 +236,10 @@ public:
 
   void visit (HIR::StructExprStructFields &stct) override
   {
-    stct.iterate ([&] (HIR::StructExprField *field) -> bool {
-      field->accept_vis (*this);
-      return true;
-    });
+    for (auto &field : stct.get_fields ())
+      {
+	field->accept_vis (*this);
+      }
 
     stct.get_struct_name ().accept_vis (*this);
     if (stct.has_struct_base ())
diff --git a/gcc/rust/lint/rust-lint-scan-deadcode.h b/gcc/rust/lint/rust-lint-scan-deadcode.h
index 464852a9f8f..152858a9e13 100644
--- a/gcc/rust/lint/rust-lint-scan-deadcode.h
+++ b/gcc/rust/lint/rust-lint-scan-deadcode.h
@@ -88,16 +88,16 @@ public:
     else
       {
 	// only warn the unused fields when in unwarned struct.
-	stct.iterate ([&] (HIR::StructField &field) -> bool {
-	  HirId field_hir_id = field.get_mappings ().get_hirid ();
-	  if (should_warn (field_hir_id))
-	    {
-	      rust_warning_at (field.get_locus (), 0,
-			       "field is never read: %<%s%>",
-			       field.get_field_name ().c_str ());
-	    }
-	  return true;
-	});
+	for (auto &field : stct.get_fields ())
+	  {
+	    HirId field_hir_id = field.get_mappings ().get_hirid ();
+	    if (should_warn (field_hir_id))
+	      {
+		rust_warning_at (field.get_locus (), 0,
+				 "field is never read: %<%s%>",
+				 field.get_field_name ().c_str ());
+	      }
+	  }
       }
   }
 
diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.h b/gcc/rust/typecheck/rust-hir-type-check-expr.h
index 28b985108cf..d9eeb4e3759 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-expr.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-expr.h
@@ -880,10 +880,11 @@ public:
   void visit (HIR::ArrayElemsValues &elems) override
   {
     std::vector<TyTy::BaseType *> types;
-    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
-      types.push_back (TypeCheckExpr::Resolve (e, false));
-      return true;
-    });
+
+    for (auto &elem : elems.get_values ())
+      {
+	types.push_back (TypeCheckExpr::Resolve (elem.get (), false));
+      }
 
     infered_array_elems
       = TyTy::TyVar::get_implicit_infer_var (root_array_expr_locus).get_tyty ();
diff --git a/gcc/rust/typecheck/rust-hir-type-check-stmt.h b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
index 5f4721b955b..3f8d17e5307 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-stmt.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
@@ -144,17 +144,18 @@ public:
     std::vector<TyTy::StructFieldType *> fields;
 
     size_t idx = 0;
-    struct_decl.iterate ([&] (HIR::TupleField &field) mutable -> bool {
-      TyTy::BaseType *field_type
-	= TypeCheckType::Resolve (field.get_field_type ().get ());
-      TyTy::StructFieldType *ty_field
-	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
-				     std::to_string (idx), field_type);
-      fields.push_back (ty_field);
-      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
-      idx++;
-      return true;
-    });
+    for (auto &field : struct_decl.get_fields ())
+      {
+	TyTy::BaseType *field_type
+	  = TypeCheckType::Resolve (field.get_field_type ().get ());
+	TyTy::StructFieldType *ty_field
+	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
+				       std::to_string (idx), field_type);
+	fields.push_back (ty_field);
+	context->insert_type (field.get_mappings (),
+			      ty_field->get_field_type ());
+	idx++;
+      }
 
     TyTy::BaseType *type
       = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
@@ -196,16 +197,17 @@ public:
       }
 
     std::vector<TyTy::StructFieldType *> fields;
-    struct_decl.iterate ([&] (HIR::StructField &field) mutable -> bool {
-      TyTy::BaseType *field_type
-	= TypeCheckType::Resolve (field.get_field_type ().get ());
-      TyTy::StructFieldType *ty_field
-	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
-				     field.get_field_name (), field_type);
-      fields.push_back (ty_field);
-      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
-      return true;
-    });
+    for (auto &field : struct_decl.get_fields ())
+      {
+	TyTy::BaseType *field_type
+	  = TypeCheckType::Resolve (field.get_field_type ().get ());
+	TyTy::StructFieldType *ty_field
+	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
+				       field.get_field_name (), field_type);
+	fields.push_back (ty_field);
+	context->insert_type (field.get_mappings (),
+			      ty_field->get_field_type ());
+      }
 
     TyTy::BaseType *type
       = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
diff --git a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
index 9fac813c46d..131149fabeb 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
@@ -79,17 +79,18 @@ public:
     std::vector<TyTy::StructFieldType *> fields;
 
     size_t idx = 0;
-    struct_decl.iterate ([&] (HIR::TupleField &field) mutable -> bool {
-      TyTy::BaseType *field_type
-	= TypeCheckType::Resolve (field.get_field_type ().get ());
-      TyTy::StructFieldType *ty_field
-	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
-				     std::to_string (idx), field_type);
-      fields.push_back (ty_field);
-      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
-      idx++;
-      return true;
-    });
+    for (auto &field : struct_decl.get_fields ())
+      {
+	TyTy::BaseType *field_type
+	  = TypeCheckType::Resolve (field.get_field_type ().get ());
+	TyTy::StructFieldType *ty_field
+	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
+				       std::to_string (idx), field_type);
+	fields.push_back (ty_field);
+	context->insert_type (field.get_mappings (),
+			      ty_field->get_field_type ());
+	idx++;
+      }
 
     TyTy::BaseType *type
       = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
@@ -136,16 +137,18 @@ public:
       }
 
     std::vector<TyTy::StructFieldType *> fields;
-    struct_decl.iterate ([&] (HIR::StructField &field) mutable -> bool {
-      TyTy::BaseType *field_type
-	= TypeCheckType::Resolve (field.get_field_type ().get ());
-      TyTy::StructFieldType *ty_field
-	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
-				     field.get_field_name (), field_type);
-      fields.push_back (ty_field);
-      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
-      return true;
-    });
+
+    for (auto &field : struct_decl.get_fields ())
+      {
+	TyTy::BaseType *field_type
+	  = TypeCheckType::Resolve (field.get_field_type ().get ());
+	TyTy::StructFieldType *ty_field
+	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
+				       field.get_field_name (), field_type);
+	fields.push_back (ty_field);
+	context->insert_type (field.get_mappings (),
+			      ty_field->get_field_type ());
+      }
 
     TyTy::BaseType *type
       = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
diff --git a/gcc/rust/typecheck/rust-hir-type-check.cc b/gcc/rust/typecheck/rust-hir-type-check.cc
index 0cd6883958d..a30f4c43a36 100644
--- a/gcc/rust/typecheck/rust-hir-type-check.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check.cc
@@ -153,20 +153,21 @@ TypeCheckStructExpr::visit (HIR::StructExprStructFields &struct_expr)
 
   std::vector<TyTy::StructFieldType *> infered_fields;
   bool ok = true;
-  struct_expr.iterate ([&] (HIR::StructExprField *field) mutable -> bool {
-    resolved_field_value_expr = nullptr;
-    field->accept_vis (*this);
-    if (resolved_field_value_expr == nullptr)
-      {
-	rust_fatal_error (field->get_locus (),
-			  "failed to resolve type for field");
-	ok = false;
-	return false;
-      }
 
-    context->insert_type (field->get_mappings (), resolved_field_value_expr);
-    return true;
-  });
+  for (auto &field : struct_expr.get_fields ())
+    {
+      resolved_field_value_expr = nullptr;
+      field->accept_vis (*this);
+      if (resolved_field_value_expr == nullptr)
+	{
+	  rust_fatal_error (field->get_locus (),
+			    "failed to resolve type for field");
+	  ok = false;
+	  break;
+	}
+
+      context->insert_type (field->get_mappings (), resolved_field_value_expr);
+    }
 
   // something failed setting up the fields
   if (!ok)
@@ -266,10 +267,8 @@ TypeCheckStructExpr::visit (HIR::StructExprStructFields &struct_expr)
       // correctly. The GIMPLE backend uses a simple algorithm that assumes each
       // assigned field in the constructor is in the same order as the field in
       // the type
-      std::vector<std::unique_ptr<HIR::StructExprField> > expr_fields
-	= struct_expr.get_fields_as_owner ();
-      for (auto &f : expr_fields)
-	f.release ();
+      for (auto &field : struct_expr.get_fields ())
+	field.release ();
 
       std::vector<std::unique_ptr<HIR::StructExprField> > ordered_fields;
       for (size_t i = 0; i < adtFieldIndexToField.size (); i++)
diff --git a/gcc/rust/typecheck/rust-tycheck-dump.h b/gcc/rust/typecheck/rust-tycheck-dump.h
index cc2e3c01110..6856d0538b8 100644
--- a/gcc/rust/typecheck/rust-tycheck-dump.h
+++ b/gcc/rust/typecheck/rust-tycheck-dump.h
@@ -162,11 +162,11 @@ public:
 
   void visit (HIR::ArrayElemsValues &elems) override
   {
-    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
-      e->accept_vis (*this);
-      dump += ",";
-      return true;
-    });
+    for (auto &elem : elems.get_values ())
+      {
+	elem->accept_vis (*this);
+	dump += ",";
+      }
   }
 
   void visit (HIR::GroupedExpr &expr) override
-- 
2.30.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Remove lambda iterators in various HIR classes
  2021-10-08 17:45 [PATCH] Remove lambda iterators in various HIR classes David Faust
@ 2021-10-08 17:49 ` David Faust
  2021-10-08 18:48   ` Marc Poulhiès
  0 siblings, 1 reply; 3+ messages in thread
From: David Faust @ 2021-10-08 17:49 UTC (permalink / raw)
  To: gcc-rust



On 10/8/21 10:45, David Faust via Gcc-rust wrote:
> This patch removes the lambda iterators used in various HIR objects.
> These iterators make interacting with the IR for static analysis more
> difficult. Instead, get_X () helpers are added for accessing elements,
> and uses of the iterators replaced with for loops.
> 
> The following objects are adjusted in this patch:
> - HIR::TupleExpr
> - HIR::StructExprField
> - HIR::StructStruct
> - HIR::TupleStruct
> 
> Fixes: #704, #705, #706, #707

This also adjusts HIR::ArrayElemsValues, fixing #703.
Must have lost those lines in the patch prep, sorry.

> ---
>   gcc/rust/backend/rust-compile-expr.h          | 20 ++++-----
>   gcc/rust/hir/tree/rust-hir-expr.h             | 32 +------------
>   gcc/rust/hir/tree/rust-hir-item.h             | 18 +-------
>   gcc/rust/lint/rust-lint-marklive.h            | 24 +++++-----
>   gcc/rust/lint/rust-lint-scan-deadcode.h       | 20 ++++-----
>   gcc/rust/typecheck/rust-hir-type-check-expr.h |  9 ++--
>   gcc/rust/typecheck/rust-hir-type-check-stmt.h | 44 +++++++++---------
>   .../typecheck/rust-hir-type-check-toplevel.h  | 45 ++++++++++---------
>   gcc/rust/typecheck/rust-hir-type-check.cc     | 33 +++++++-------
>   gcc/rust/typecheck/rust-tycheck-dump.h        | 10 ++---
>   10 files changed, 107 insertions(+), 148 deletions(-)
> 
> diff --git a/gcc/rust/backend/rust-compile-expr.h b/gcc/rust/backend/rust-compile-expr.h
> index eb245dce5be..7c4046680e9 100644
> --- a/gcc/rust/backend/rust-compile-expr.h
> +++ b/gcc/rust/backend/rust-compile-expr.h
> @@ -412,11 +412,11 @@ public:
>   
>     void visit (HIR::ArrayElemsValues &elems) override
>     {
> -    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
> -      Bexpression *translated_expr = CompileExpr::Compile (e, ctx);
> -      constructor.push_back (translated_expr);
> -      return true;
> -    });
> +    for (auto &elem : elems.get_values ())
> +      {
> +	Bexpression *translated_expr = CompileExpr::Compile (elem.get (), ctx);
> +	constructor.push_back (translated_expr);
> +      }
>     }
>   
>     void visit (HIR::ArrayElemsCopied &elems) override
> @@ -646,11 +646,11 @@ public:
>       // this assumes all fields are in order from type resolution and if a base
>       // struct was specified those fields are filed via accesors
>       std::vector<Bexpression *> vals;
> -    struct_expr.iterate ([&] (HIR::StructExprField *field) mutable -> bool {
> -      Bexpression *expr = CompileStructExprField::Compile (field, ctx);
> -      vals.push_back (expr);
> -      return true;
> -    });
> +    for (auto &field : struct_expr.get_fields ())
> +      {
> +	Bexpression *expr = CompileStructExprField::Compile (field.get (), ctx);
> +	vals.push_back (expr);
> +      }
>   
>       translated
>         = ctx->get_backend ()->constructor_expression (type, vals,
> diff --git a/gcc/rust/hir/tree/rust-hir-expr.h b/gcc/rust/hir/tree/rust-hir-expr.h
> index 05bc1f9f055..d9958a153be 100644
> --- a/gcc/rust/hir/tree/rust-hir-expr.h
> +++ b/gcc/rust/hir/tree/rust-hir-expr.h
> @@ -796,14 +796,7 @@ public:
>   
>     size_t get_num_elements () const { return values.size (); }
>   
> -  void iterate (std::function<bool (Expr *)> cb)
> -  {
> -    for (auto it = values.begin (); it != values.end (); it++)
> -      {
> -	if (!cb ((*it).get ()))
> -	  return;
> -      }
> -  }
> +  std::vector<std::unique_ptr<Expr>>& get_values () { return values; }
>   
>   protected:
>     ArrayElemsValues *clone_array_elems_impl () const override
> @@ -1070,15 +1063,6 @@ public:
>   
>     bool is_unit () const { return tuple_elems.size () == 0; }
>   
> -  void iterate (std::function<bool (Expr *)> cb)
> -  {
> -    for (auto &tuple_elem : tuple_elems)
> -      {
> -	if (!cb (tuple_elem.get ()))
> -	  return;
> -      }
> -  }
> -
>   protected:
>     /* Use covariance to implement clone function as returning this object rather
>      * than base */
> @@ -1491,15 +1475,6 @@ public:
>   
>     void accept_vis (HIRVisitor &vis) override;
>   
> -  void iterate (std::function<bool (StructExprField *)> cb)
> -  {
> -    for (auto &field : fields)
> -      {
> -	if (!cb (field.get ()))
> -	  return;
> -      }
> -  }
> -
>     std::vector<std::unique_ptr<StructExprField> > &get_fields ()
>     {
>       return fields;
> @@ -1510,11 +1485,6 @@ public:
>       return fields;
>     };
>   
> -  std::vector<std::unique_ptr<StructExprField> > get_fields_as_owner ()
> -  {
> -    return std::move (fields);
> -  };
> -
>     void set_fields_as_owner (
>       std::vector<std::unique_ptr<StructExprField> > new_fields)
>     {
> diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
> index 8cd7a01b2e8..f5119c3dc77 100644
> --- a/gcc/rust/hir/tree/rust-hir-item.h
> +++ b/gcc/rust/hir/tree/rust-hir-item.h
> @@ -1492,14 +1492,7 @@ public:
>   
>     void accept_vis (HIRVisitor &vis) override;
>   
> -  void iterate (std::function<bool (StructField &)> cb)
> -  {
> -    for (auto &field : fields)
> -      {
> -	if (!cb (field))
> -	  return;
> -      }
> -  }
> +  std::vector<StructField>& get_fields() { return fields; }
>   
>   protected:
>     /* Use covariance to implement clone function as returning this object
> @@ -1610,15 +1603,6 @@ public:
>     std::vector<TupleField> &get_fields () { return fields; }
>     const std::vector<TupleField> &get_fields () const { return fields; }
>   
> -  void iterate (std::function<bool (TupleField &)> cb)
> -  {
> -    for (auto &field : fields)
> -      {
> -	if (!cb (field))
> -	  return;
> -      }
> -  }
> -
>   protected:
>     /* Use covariance to implement clone function as returning this object
>      * rather than base */
> diff --git a/gcc/rust/lint/rust-lint-marklive.h b/gcc/rust/lint/rust-lint-marklive.h
> index 062bb96bc0f..cd72ef0243d 100644
> --- a/gcc/rust/lint/rust-lint-marklive.h
> +++ b/gcc/rust/lint/rust-lint-marklive.h
> @@ -81,18 +81,18 @@ public:
>   
>     void visit (HIR::ArrayElemsValues &expr) override
>     {
> -    expr.iterate ([&] (HIR::Expr *expr) mutable -> bool {
> -      expr->accept_vis (*this);
> -      return true;
> -    });
> +    for (auto &elem : expr.get_values ())
> +      {
> +	elem->accept_vis (*this);
> +      }
>     }
>   
>     void visit (HIR::TupleExpr &expr) override
>     {
> -    expr.iterate ([&] (HIR::Expr *expr) mutable -> bool {
> -      expr->accept_vis (*this);
> -      return true;
> -    });
> +    for (auto &elem : expr.get_tuple_elems ())
> +      {
> +	elem->accept_vis (*this);
> +      }
>     }
>   
>     void visit (HIR::BlockExpr &expr) override
> @@ -236,10 +236,10 @@ public:
>   
>     void visit (HIR::StructExprStructFields &stct) override
>     {
> -    stct.iterate ([&] (HIR::StructExprField *field) -> bool {
> -      field->accept_vis (*this);
> -      return true;
> -    });
> +    for (auto &field : stct.get_fields ())
> +      {
> +	field->accept_vis (*this);
> +      }
>   
>       stct.get_struct_name ().accept_vis (*this);
>       if (stct.has_struct_base ())
> diff --git a/gcc/rust/lint/rust-lint-scan-deadcode.h b/gcc/rust/lint/rust-lint-scan-deadcode.h
> index 464852a9f8f..152858a9e13 100644
> --- a/gcc/rust/lint/rust-lint-scan-deadcode.h
> +++ b/gcc/rust/lint/rust-lint-scan-deadcode.h
> @@ -88,16 +88,16 @@ public:
>       else
>         {
>   	// only warn the unused fields when in unwarned struct.
> -	stct.iterate ([&] (HIR::StructField &field) -> bool {
> -	  HirId field_hir_id = field.get_mappings ().get_hirid ();
> -	  if (should_warn (field_hir_id))
> -	    {
> -	      rust_warning_at (field.get_locus (), 0,
> -			       "field is never read: %<%s%>",
> -			       field.get_field_name ().c_str ());
> -	    }
> -	  return true;
> -	});
> +	for (auto &field : stct.get_fields ())
> +	  {
> +	    HirId field_hir_id = field.get_mappings ().get_hirid ();
> +	    if (should_warn (field_hir_id))
> +	      {
> +		rust_warning_at (field.get_locus (), 0,
> +				 "field is never read: %<%s%>",
> +				 field.get_field_name ().c_str ());
> +	      }
> +	  }
>         }
>     }
>   
> diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.h b/gcc/rust/typecheck/rust-hir-type-check-expr.h
> index 28b985108cf..d9eeb4e3759 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check-expr.h
> +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.h
> @@ -880,10 +880,11 @@ public:
>     void visit (HIR::ArrayElemsValues &elems) override
>     {
>       std::vector<TyTy::BaseType *> types;
> -    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
> -      types.push_back (TypeCheckExpr::Resolve (e, false));
> -      return true;
> -    });
> +
> +    for (auto &elem : elems.get_values ())
> +      {
> +	types.push_back (TypeCheckExpr::Resolve (elem.get (), false));
> +      }
>   
>       infered_array_elems
>         = TyTy::TyVar::get_implicit_infer_var (root_array_expr_locus).get_tyty ();
> diff --git a/gcc/rust/typecheck/rust-hir-type-check-stmt.h b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> index 5f4721b955b..3f8d17e5307 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> +++ b/gcc/rust/typecheck/rust-hir-type-check-stmt.h
> @@ -144,17 +144,18 @@ public:
>       std::vector<TyTy::StructFieldType *> fields;
>   
>       size_t idx = 0;
> -    struct_decl.iterate ([&] (HIR::TupleField &field) mutable -> bool {
> -      TyTy::BaseType *field_type
> -	= TypeCheckType::Resolve (field.get_field_type ().get ());
> -      TyTy::StructFieldType *ty_field
> -	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> -				     std::to_string (idx), field_type);
> -      fields.push_back (ty_field);
> -      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
> -      idx++;
> -      return true;
> -    });
> +    for (auto &field : struct_decl.get_fields ())
> +      {
> +	TyTy::BaseType *field_type
> +	  = TypeCheckType::Resolve (field.get_field_type ().get ());
> +	TyTy::StructFieldType *ty_field
> +	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> +				       std::to_string (idx), field_type);
> +	fields.push_back (ty_field);
> +	context->insert_type (field.get_mappings (),
> +			      ty_field->get_field_type ());
> +	idx++;
> +      }
>   
>       TyTy::BaseType *type
>         = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
> @@ -196,16 +197,17 @@ public:
>         }
>   
>       std::vector<TyTy::StructFieldType *> fields;
> -    struct_decl.iterate ([&] (HIR::StructField &field) mutable -> bool {
> -      TyTy::BaseType *field_type
> -	= TypeCheckType::Resolve (field.get_field_type ().get ());
> -      TyTy::StructFieldType *ty_field
> -	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> -				     field.get_field_name (), field_type);
> -      fields.push_back (ty_field);
> -      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
> -      return true;
> -    });
> +    for (auto &field : struct_decl.get_fields ())
> +      {
> +	TyTy::BaseType *field_type
> +	  = TypeCheckType::Resolve (field.get_field_type ().get ());
> +	TyTy::StructFieldType *ty_field
> +	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> +				       field.get_field_name (), field_type);
> +	fields.push_back (ty_field);
> +	context->insert_type (field.get_mappings (),
> +			      ty_field->get_field_type ());
> +      }
>   
>       TyTy::BaseType *type
>         = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
> diff --git a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> index 9fac813c46d..131149fabeb 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> +++ b/gcc/rust/typecheck/rust-hir-type-check-toplevel.h
> @@ -79,17 +79,18 @@ public:
>       std::vector<TyTy::StructFieldType *> fields;
>   
>       size_t idx = 0;
> -    struct_decl.iterate ([&] (HIR::TupleField &field) mutable -> bool {
> -      TyTy::BaseType *field_type
> -	= TypeCheckType::Resolve (field.get_field_type ().get ());
> -      TyTy::StructFieldType *ty_field
> -	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> -				     std::to_string (idx), field_type);
> -      fields.push_back (ty_field);
> -      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
> -      idx++;
> -      return true;
> -    });
> +    for (auto &field : struct_decl.get_fields ())
> +      {
> +	TyTy::BaseType *field_type
> +	  = TypeCheckType::Resolve (field.get_field_type ().get ());
> +	TyTy::StructFieldType *ty_field
> +	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> +				       std::to_string (idx), field_type);
> +	fields.push_back (ty_field);
> +	context->insert_type (field.get_mappings (),
> +			      ty_field->get_field_type ());
> +	idx++;
> +      }
>   
>       TyTy::BaseType *type
>         = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
> @@ -136,16 +137,18 @@ public:
>         }
>   
>       std::vector<TyTy::StructFieldType *> fields;
> -    struct_decl.iterate ([&] (HIR::StructField &field) mutable -> bool {
> -      TyTy::BaseType *field_type
> -	= TypeCheckType::Resolve (field.get_field_type ().get ());
> -      TyTy::StructFieldType *ty_field
> -	= new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> -				     field.get_field_name (), field_type);
> -      fields.push_back (ty_field);
> -      context->insert_type (field.get_mappings (), ty_field->get_field_type ());
> -      return true;
> -    });
> +
> +    for (auto &field : struct_decl.get_fields ())
> +      {
> +	TyTy::BaseType *field_type
> +	  = TypeCheckType::Resolve (field.get_field_type ().get ());
> +	TyTy::StructFieldType *ty_field
> +	  = new TyTy::StructFieldType (field.get_mappings ().get_hirid (),
> +				       field.get_field_name (), field_type);
> +	fields.push_back (ty_field);
> +	context->insert_type (field.get_mappings (),
> +			      ty_field->get_field_type ());
> +      }
>   
>       TyTy::BaseType *type
>         = new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
> diff --git a/gcc/rust/typecheck/rust-hir-type-check.cc b/gcc/rust/typecheck/rust-hir-type-check.cc
> index 0cd6883958d..a30f4c43a36 100644
> --- a/gcc/rust/typecheck/rust-hir-type-check.cc
> +++ b/gcc/rust/typecheck/rust-hir-type-check.cc
> @@ -153,20 +153,21 @@ TypeCheckStructExpr::visit (HIR::StructExprStructFields &struct_expr)
>   
>     std::vector<TyTy::StructFieldType *> infered_fields;
>     bool ok = true;
> -  struct_expr.iterate ([&] (HIR::StructExprField *field) mutable -> bool {
> -    resolved_field_value_expr = nullptr;
> -    field->accept_vis (*this);
> -    if (resolved_field_value_expr == nullptr)
> -      {
> -	rust_fatal_error (field->get_locus (),
> -			  "failed to resolve type for field");
> -	ok = false;
> -	return false;
> -      }
>   
> -    context->insert_type (field->get_mappings (), resolved_field_value_expr);
> -    return true;
> -  });
> +  for (auto &field : struct_expr.get_fields ())
> +    {
> +      resolved_field_value_expr = nullptr;
> +      field->accept_vis (*this);
> +      if (resolved_field_value_expr == nullptr)
> +	{
> +	  rust_fatal_error (field->get_locus (),
> +			    "failed to resolve type for field");
> +	  ok = false;
> +	  break;
> +	}
> +
> +      context->insert_type (field->get_mappings (), resolved_field_value_expr);
> +    }
>   
>     // something failed setting up the fields
>     if (!ok)
> @@ -266,10 +267,8 @@ TypeCheckStructExpr::visit (HIR::StructExprStructFields &struct_expr)
>         // correctly. The GIMPLE backend uses a simple algorithm that assumes each
>         // assigned field in the constructor is in the same order as the field in
>         // the type
> -      std::vector<std::unique_ptr<HIR::StructExprField> > expr_fields
> -	= struct_expr.get_fields_as_owner ();
> -      for (auto &f : expr_fields)
> -	f.release ();
> +      for (auto &field : struct_expr.get_fields ())
> +	field.release ();
>   
>         std::vector<std::unique_ptr<HIR::StructExprField> > ordered_fields;
>         for (size_t i = 0; i < adtFieldIndexToField.size (); i++)
> diff --git a/gcc/rust/typecheck/rust-tycheck-dump.h b/gcc/rust/typecheck/rust-tycheck-dump.h
> index cc2e3c01110..6856d0538b8 100644
> --- a/gcc/rust/typecheck/rust-tycheck-dump.h
> +++ b/gcc/rust/typecheck/rust-tycheck-dump.h
> @@ -162,11 +162,11 @@ public:
>   
>     void visit (HIR::ArrayElemsValues &elems) override
>     {
> -    elems.iterate ([&] (HIR::Expr *e) mutable -> bool {
> -      e->accept_vis (*this);
> -      dump += ",";
> -      return true;
> -    });
> +    for (auto &elem : elems.get_values ())
> +      {
> +	elem->accept_vis (*this);
> +	dump += ",";
> +      }
>     }
>   
>     void visit (HIR::GroupedExpr &expr) override
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Remove lambda iterators in various HIR classes
  2021-10-08 17:49 ` David Faust
@ 2021-10-08 18:48   ` Marc Poulhiès
  0 siblings, 0 replies; 3+ messages in thread
From: Marc Poulhiès @ 2021-10-08 18:48 UTC (permalink / raw)
  To: David Faust via Gcc-rust


David Faust via Gcc-rust <gcc-rust@gcc.gnu.org> writes:

> On 10/8/21 10:45, David Faust via Gcc-rust wrote:
>> This patch removes the lambda iterators used in various HIR objects.
>> These iterators make interacting with the IR for static analysis more
>> difficult. Instead, get_X () helpers are added for accessing elements,
>> and uses of the iterators replaced with for loops.
>> The following objects are adjusted in this patch:
>> - HIR::TupleExpr
>> - HIR::StructExprField
>> - HIR::StructStruct
>> - HIR::TupleStruct
>> Fixes: #704, #705, #706, #707
>
> This also adjusts HIR::ArrayElemsValues, fixing #703.
> Must have lost those lines in the patch prep, sorry.

Hi David,

Thank you for this patch !

As gccrs is relying on github, feel free to open a pull request directly
(if you can/want).

I took care of it (and added the #703 while I was at it), and you can
see that there are some small issues with the indentation:

https://github.com/Rust-GCC/gccrs/pull/726/checks?check_run_id=3841896345

You need to be logged-in to see the results. If you don't have an
account, you can still apply clang-format by hand :)

Marc

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-08 18:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-08 17:45 [PATCH] Remove lambda iterators in various HIR classes David Faust
2021-10-08 17:49 ` David Faust
2021-10-08 18:48   ` Marc Poulhiès

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).