Skip to content

Commit

Permalink
Merge #710 #727
Browse files Browse the repository at this point in the history
710: Ensure for Coercion Sites we emit the code nessecary r=philberty a=philberty

Coercion sites in Rust can require extra code generation for
CallExpressions arguments for example. This ensures we detect
those cases and emit the extra code necessary. Please read the individual
commit messages for more detail on how this works.

Fixes #700 #708 #709  

727: Remove lambda iterators in various HIR classes r=philberty a=dafaust

(This is a revision of #726 with formatting fixes)

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::ArrayElemsValues
- HIR::TupleExpr
- HIR::StructExprField
- HIR::StructStruct
- HIR::TupleStruct

Fixes: #703 
Fixes: #704 
Fixes: #705 
Fixes: #706 
Fixes: #707

Co-authored-by: Philip Herron <philip.herron@embecosm.com>
Co-authored-by: David Faust <david.faust@oracle.com>
  • Loading branch information
3 people authored Oct 13, 2021
3 parents 6355d05 + f6a04f3 + 85338a7 commit 7a34177
Show file tree
Hide file tree
Showing 17 changed files with 430 additions and 300 deletions.
42 changes: 29 additions & 13 deletions gcc/rust/backend/rust-compile-expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,28 @@ class CompileExpr : public HIRCompileBase
void visit (HIR::AssignmentExpr &expr) override
{
fncontext fn = ctx->peek_fn ();
auto lhs = CompileExpr::Compile (expr.get_lhs (), ctx);
auto rhs = CompileExpr::Compile (expr.get_rhs (), ctx);
auto lvalue = CompileExpr::Compile (expr.get_lhs (), ctx);
auto rvalue = CompileExpr::Compile (expr.get_rhs (), ctx);

// assignments are coercion sites so lets convert the rvalue if necessary
TyTy::BaseType *expected = nullptr;
TyTy::BaseType *actual = nullptr;

bool ok;
ok = ctx->get_tyctx ()->lookup_type (
expr.get_lhs ()->get_mappings ().get_hirid (), &expected);
rust_assert (ok);

ok = ctx->get_tyctx ()->lookup_type (
expr.get_rhs ()->get_mappings ().get_hirid (), &actual);
rust_assert (ok);

rvalue = coercion_site (rvalue, actual, expected, expr.get_locus ());

Bstatement *assignment
= ctx->get_backend ()->assignment_statement (fn.fndecl, lhs, rhs,
= ctx->get_backend ()->assignment_statement (fn.fndecl, lvalue, rvalue,
expr.get_locus ());

ctx->add_statement (assignment);
}

Expand Down Expand Up @@ -412,11 +428,11 @@ class CompileExpr : public HIRCompileBase

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
Expand Down Expand Up @@ -646,11 +662,11 @@ class CompileExpr : public HIRCompileBase
// 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,
Expand Down
142 changes: 111 additions & 31 deletions gcc/rust/backend/rust-compile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,39 +69,120 @@ CompileExpr::visit (HIR::CallExpr &expr)
|| tyty->get_kind () == TyTy::TypeKind::FNPTR;
if (!is_fn)
{
Btype *type = TyTyResolveCompile::compile (ctx, tyty);
rust_assert (tyty->get_kind () == TyTy::TypeKind::ADT);
TyTy::ADTType *adt = static_cast<TyTy::ADTType *> (tyty);
Btype *compiled_adt_type = TyTyResolveCompile::compile (ctx, tyty);

// 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;
expr.iterate_params ([&] (HIR::Expr *argument) mutable -> bool {
Bexpression *e = CompileExpr::Compile (argument, ctx);
vals.push_back (e);
return true;
});
for (size_t i = 0; i < expr.get_arguments ().size (); i++)
{
auto &argument = expr.get_arguments ().at (i);
auto rvalue = CompileExpr::Compile (argument.get (), ctx);

// assignments are coercion sites so lets convert the rvalue if
// necessary
auto respective_field = adt->get_field (i);
auto expected = respective_field->get_field_type ();

TyTy::BaseType *actual = nullptr;
bool ok = ctx->get_tyctx ()->lookup_type (
argument->get_mappings ().get_hirid (), &actual);
rust_assert (ok);

// coerce it if required
rvalue = coercion_site (rvalue, actual, expected, expr.get_locus ());

// add it to the list
vals.push_back (rvalue);
}

translated
= ctx->get_backend ()->constructor_expression (type, vals, -1,
expr.get_locus ());
= ctx->get_backend ()->constructor_expression (compiled_adt_type, vals,
-1, expr.get_locus ());
}
else
{
// must be a call to a function
Bexpression *fn = CompileExpr::Compile (expr.get_fnexpr (), ctx);
rust_assert (fn != nullptr);
auto get_parameter_tyty_at_index
= [] (const TyTy::BaseType *base, size_t index,
TyTy::BaseType **result) -> bool {
bool is_fn = base->get_kind () == TyTy::TypeKind::FNDEF
|| base->get_kind () == TyTy::TypeKind::FNPTR;
rust_assert (is_fn);

if (base->get_kind () == TyTy::TypeKind::FNPTR)
{
const TyTy::FnPtr *fn = static_cast<const TyTy::FnPtr *> (base);
*result = fn->param_at (index);

return true;
}

const TyTy::FnType *fn = static_cast<const TyTy::FnType *> (base);
auto param = fn->param_at (index);
*result = param.second;

std::vector<Bexpression *> args;
expr.iterate_params ([&] (HIR::Expr *p) mutable -> bool {
Bexpression *compiled_expr = CompileExpr::Compile (p, ctx);
rust_assert (compiled_expr != nullptr);
args.push_back (compiled_expr);
return true;
});
};

bool is_varadic = false;
if (tyty->get_kind () == TyTy::TypeKind::FNDEF)
{
const TyTy::FnType *fn = static_cast<const TyTy::FnType *> (tyty);
is_varadic = fn->is_varadic ();
}

size_t required_num_args;
if (tyty->get_kind () == TyTy::TypeKind::FNDEF)
{
const TyTy::FnType *fn = static_cast<const TyTy::FnType *> (tyty);
required_num_args = fn->num_params ();
}
else
{
const TyTy::FnPtr *fn = static_cast<const TyTy::FnPtr *> (tyty);
required_num_args = fn->num_params ();
}

std::vector<Bexpression *> args;
for (size_t i = 0; i < expr.get_arguments ().size (); i++)
{
auto &argument = expr.get_arguments ().at (i);
auto rvalue = CompileExpr::Compile (argument.get (), ctx);

if (is_varadic && i >= required_num_args)
{
args.push_back (rvalue);
continue;
}

// assignments are coercion sites so lets convert the rvalue if
// necessary
bool ok;
TyTy::BaseType *expected = nullptr;
ok = get_parameter_tyty_at_index (tyty, i, &expected);
rust_assert (ok);

TyTy::BaseType *actual = nullptr;
ok = ctx->get_tyctx ()->lookup_type (
argument->get_mappings ().get_hirid (), &actual);
rust_assert (ok);

// coerce it if required
rvalue = coercion_site (rvalue, actual, expected, expr.get_locus ());

// add it to the list
args.push_back (rvalue);
}

// must be a call to a function
auto fn_address = CompileExpr::Compile (expr.get_fnexpr (), ctx);
auto fncontext = ctx->peek_fn ();
translated
= ctx->get_backend ()->call_expression (fncontext.fndecl, fn, args,
nullptr, expr.get_locus ());
= ctx->get_backend ()->call_expression (fncontext.fndecl, fn_address,
args, nullptr,
expr.get_locus ());
}
}

Expand Down Expand Up @@ -224,12 +305,12 @@ CompileExpr::visit (HIR::MethodCallExpr &expr)

std::vector<Bexpression *> args;
args.push_back (self_argument);
expr.iterate_params ([&] (HIR::Expr *p) mutable -> bool {
Bexpression *compiled_expr = CompileExpr::Compile (p, ctx);
rust_assert (compiled_expr != nullptr);
args.push_back (compiled_expr);
return true;
});
for (auto &argument : expr.get_arguments ())
{
Bexpression *compiled_expr
= CompileExpr::Compile (argument.get (), ctx);
args.push_back (compiled_expr);
}

Bexpression *fn_expr
= ctx->get_backend ()->var_expression (fn_convert_expr_tmp,
Expand Down Expand Up @@ -414,12 +495,11 @@ CompileExpr::visit (HIR::MethodCallExpr &expr)
args.push_back (self);

// normal args
expr.iterate_params ([&] (HIR::Expr *p) mutable -> bool {
Bexpression *compiled_expr = CompileExpr::Compile (p, ctx);
rust_assert (compiled_expr != nullptr);
args.push_back (compiled_expr);
return true;
});
for (auto &argument : expr.get_arguments ())
{
Bexpression *compiled_expr = CompileExpr::Compile (argument.get (), ctx);
args.push_back (compiled_expr);
}

auto fncontext = ctx->peek_fn ();
translated
Expand Down
62 changes: 9 additions & 53 deletions gcc/rust/hir/tree/rust-hir-expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -796,14 +796,7 @@ class ArrayElemsValues : public ArrayElems

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
Expand Down Expand Up @@ -1070,15 +1063,6 @@ class TupleExpr : public ExprWithoutBlock

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 */
Expand Down Expand Up @@ -1491,15 +1475,6 @@ class StructExprStructFields : public StructExprStruct

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;
Expand All @@ -1510,11 +1485,6 @@ class StructExprStructFields : public StructExprStruct
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)
{
Expand Down Expand Up @@ -1578,14 +1548,10 @@ class StructExprStructBase : public StructExprStruct
}
};

// Forward decl for Function - used in CallExpr
class Function;

// Function call expression HIR node
class CallExpr : public ExprWithoutBlock
{
std::unique_ptr<Expr> function;
// inlined form of CallParams
std::vector<std::unique_ptr<Expr> > params;

Location locus;
Expand Down Expand Up @@ -1642,13 +1608,11 @@ class CallExpr : public ExprWithoutBlock

size_t num_params () const { return params.size (); }

void iterate_params (std::function<bool (Expr *)> cb)
std::vector<std::unique_ptr<Expr> > &get_arguments () { return params; }

const std::vector<std::unique_ptr<Expr> > &get_arguments () const
{
for (auto &param : params)
{
if (!cb (param.get ()))
return;
}
return params;
}

protected:
Expand Down Expand Up @@ -1731,21 +1695,13 @@ class MethodCallExpr : public ExprWithoutBlock

PathExprSegment get_method_name () const { return method_name; };

std::vector<std::unique_ptr<Expr> > &get_params () { return params; }
const std::vector<std::unique_ptr<Expr> > &get_params () const
{
return params;
}

size_t num_params () const { return params.size (); }

void iterate_params (std::function<bool (Expr *)> cb)
std::vector<std::unique_ptr<Expr> > &get_arguments () { return params; }

const std::vector<std::unique_ptr<Expr> > &get_arguments () const
{
for (auto &param : params)
{
if (!cb (param.get ()))
return;
}
return params;
}

protected:
Expand Down
18 changes: 1 addition & 17 deletions gcc/rust/hir/tree/rust-hir-item.h
Original file line number Diff line number Diff line change
Expand Up @@ -1492,14 +1492,7 @@ class StructStruct : public Struct

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
Expand Down Expand Up @@ -1610,15 +1603,6 @@ class TupleStruct : public Struct
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 */
Expand Down
Loading

0 comments on commit 7a34177

Please sign in to comment.