From: David Faust <david.faust@oracle.com>
To: gcc-rust@gcc.gnu.org
Subject: Re: [PATCH] Remove lambda iterators in various HIR classes
Date: Fri, 8 Oct 2021 10:49:13 -0700 [thread overview]
Message-ID: <883ae628-cbbe-946f-a5f0-32ed2e951975@oracle.com> (raw)
In-Reply-To: <20211008174507.10534-1-david.faust@oracle.com>
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
>
next prev parent reply other threads:[~2021-10-08 17:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-08 17:45 David Faust
2021-10-08 17:49 ` David Faust [this message]
2021-10-08 18:48 ` Marc Poulhiès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=883ae628-cbbe-946f-a5f0-32ed2e951975@oracle.com \
--to=david.faust@oracle.com \
--cc=gcc-rust@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).