From 579d76f771bd18c27a527df729d0dcf460c08744 Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Tue, 5 Oct 2021 14:19:05 +0100 Subject: [PATCH 1/5] Remove lambda iterator from HIR::CallExpr This removes the bad code style lambda iterators for arguments. They are a bad design choice for static analysis code since the user of the api looses scope to break/return outside from the caller api. This will need to be added to a style-guide in the future. Fixes: #708 --- gcc/rust/backend/rust-compile.cc | 22 ++-- gcc/rust/hir/tree/rust-hir-expr.h | 14 +-- gcc/rust/lint/rust-lint-marklive.h | 6 +- gcc/rust/typecheck/rust-tyty.cc | 131 ++++++++++---------- gcc/testsuite/rust/compile/tuple_struct3.rs | 5 +- 5 files changed, 87 insertions(+), 91 deletions(-) diff --git a/gcc/rust/backend/rust-compile.cc b/gcc/rust/backend/rust-compile.cc index 58c679f74ff2..d6c606688211 100644 --- a/gcc/rust/backend/rust-compile.cc +++ b/gcc/rust/backend/rust-compile.cc @@ -74,11 +74,11 @@ CompileExpr::visit (HIR::CallExpr &expr) // 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 vals; - expr.iterate_params ([&] (HIR::Expr *argument) mutable -> bool { - Bexpression *e = CompileExpr::Compile (argument, ctx); - vals.push_back (e); - return true; - }); + for (auto &argument : expr.get_arguments ()) + { + Bexpression *e = CompileExpr::Compile (argument.get (), ctx); + vals.push_back (e); + } translated = ctx->get_backend ()->constructor_expression (type, vals, -1, @@ -91,12 +91,12 @@ CompileExpr::visit (HIR::CallExpr &expr) rust_assert (fn != nullptr); std::vector 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 diff --git a/gcc/rust/hir/tree/rust-hir-expr.h b/gcc/rust/hir/tree/rust-hir-expr.h index 05bc1f9f0554..d66640d6f397 100644 --- a/gcc/rust/hir/tree/rust-hir-expr.h +++ b/gcc/rust/hir/tree/rust-hir-expr.h @@ -1578,14 +1578,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 function; - // inlined form of CallParams std::vector > params; Location locus; @@ -1642,13 +1638,11 @@ class CallExpr : public ExprWithoutBlock size_t num_params () const { return params.size (); } - void iterate_params (std::function cb) + std::vector > &get_arguments () { return params; } + + const std::vector > &get_arguments () const { - for (auto ¶m : params) - { - if (!cb (param.get ())) - return; - } + return params; } protected: diff --git a/gcc/rust/lint/rust-lint-marklive.h b/gcc/rust/lint/rust-lint-marklive.h index 062bb96bc0f3..481c956ac411 100644 --- a/gcc/rust/lint/rust-lint-marklive.h +++ b/gcc/rust/lint/rust-lint-marklive.h @@ -165,10 +165,8 @@ class MarkLive : public MarkLiveBase void visit (HIR::CallExpr &expr) override { expr.get_fnexpr ()->accept_vis (*this); - expr.iterate_params ([&] (HIR::Expr *expr) -> bool { - expr->accept_vis (*this); - return true; - }); + for (auto &argument : expr.get_arguments ()) + argument->accept_vis (*this); } void visit (HIR::ArithmeticOrLogicalExpr &expr) override diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index f4ce501c48c3..8de9d9d42a8f 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -2383,27 +2383,28 @@ TypeCheckCallExpr::visit (ADTType &type) } size_t i = 0; - call.iterate_params ([&] (HIR::Expr *p) mutable -> bool { - StructFieldType *field = type.get_field (i); - BaseType *field_tyty = field->get_field_type (); + for (auto &argument : call.get_arguments ()) + { + StructFieldType *field = type.get_field (i); + BaseType *field_tyty = field->get_field_type (); - BaseType *arg = Resolver::TypeCheckExpr::Resolve (p, false); - if (arg->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (p->get_locus (), "failed to resolve argument type"); - return false; - } + BaseType *arg = Resolver::TypeCheckExpr::Resolve (argument.get (), false); + if (arg->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at (argument->get_locus (), + "failed to resolve argument type"); + return; + } - auto res = field_tyty->coerce (arg); - if (res->get_kind () == TyTy::TypeKind::ERROR) - { - return false; - } + auto res = field_tyty->coerce (arg); + if (res->get_kind () == TyTy::TypeKind::ERROR) + { + return; + } - delete res; - i++; - return true; - }); + delete res; + i++; + } if (i != call.num_params ()) { @@ -2441,35 +2442,37 @@ TypeCheckCallExpr::visit (FnType &type) } size_t i = 0; - call.iterate_params ([&] (HIR::Expr *param) mutable -> bool { - auto argument_expr_tyty = Resolver::TypeCheckExpr::Resolve (param, false); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "failed to resolve type for argument expr in CallExpr"); - return false; - } + for (auto &argument : call.get_arguments ()) + { + auto argument_expr_tyty + = Resolver::TypeCheckExpr::Resolve (argument.get (), false); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at ( + argument->get_locus (), + "failed to resolve type for argument expr in CallExpr"); + return; + } - auto resolved_argument_type = argument_expr_tyty; + auto resolved_argument_type = argument_expr_tyty; - // it might be a varadic function - if (i < type.num_params ()) - { - auto fnparam = type.param_at (i); - resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "Type Resolution failure on parameter"); - return false; - } - } + // it might be a varadic function + if (i < type.num_params ()) + { + auto fnparam = type.param_at (i); + resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at (argument->get_locus (), + "Type Resolution failure on parameter"); + return; + } + } - context->insert_type (param->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), resolved_argument_type); - i++; - return true; - }); + i++; + } if (i < call.num_params ()) { @@ -2505,29 +2508,31 @@ TypeCheckCallExpr::visit (FnPtr &type) } size_t i = 0; - call.iterate_params ([&] (HIR::Expr *param) mutable -> bool { - auto fnparam = type.param_at (i); - auto argument_expr_tyty = Resolver::TypeCheckExpr::Resolve (param, false); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "failed to resolve type for argument expr in CallExpr"); - return false; - } + for (auto &argument : call.get_arguments ()) + { + auto fnparam = type.param_at (i); + auto argument_expr_tyty + = Resolver::TypeCheckExpr::Resolve (argument.get (), false); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at ( + argument->get_locus (), + "failed to resolve type for argument expr in CallExpr"); + return; + } - auto resolved_argument_type = fnparam->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "Type Resolution failure on parameter"); - return false; - } + auto resolved_argument_type = fnparam->coerce (argument_expr_tyty); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at (argument->get_locus (), + "Type Resolution failure on parameter"); + return; + } - context->insert_type (param->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), resolved_argument_type); - i++; - return true; - }); + i++; + } if (i != call.num_params ()) { diff --git a/gcc/testsuite/rust/compile/tuple_struct3.rs b/gcc/testsuite/rust/compile/tuple_struct3.rs index 1af72a28551d..a70306df852b 100644 --- a/gcc/testsuite/rust/compile/tuple_struct3.rs +++ b/gcc/testsuite/rust/compile/tuple_struct3.rs @@ -3,7 +3,6 @@ struct Foo(i32, i32, bool); fn main() { let c = Foo(1, 2f32, true); // { dg-error "expected .i32. got .f32." "" { target *-*-* } .-1 } - // { dg-error "unexpected number of arguments 1 expected 3" "" { target *-*-* } .-2 } - // { dg-error "failed to lookup type to CallExpr" "" { target *-*-* } .-3 } - // { dg-error "failed to type resolve expression" "" { target *-*-* } .-4 } + // { dg-error "failed to lookup type to CallExpr" "" { target *-*-* } .-2 } + // { dg-error "failed to type resolve expression" "" { target *-*-* } .-3 } } From 6cd07341b1057961bebc8c11d70909ccac781113 Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Tue, 5 Oct 2021 14:30:58 +0100 Subject: [PATCH 2/5] Remove lambda iterator from HIR::MethodCallExpr This removes the bad code style lambda iterators for arguments. They are a bad design choice for static analysis code since the user of the api looses scope to break/return outside from the caller api. This will need to be added to a style-guide in the future. Fixes: #709 --- gcc/rust/backend/rust-compile.cc | 23 ++++++++-------- gcc/rust/hir/tree/rust-hir-expr.h | 16 +++-------- gcc/rust/lint/rust-lint-marklive.cc | 6 ++--- gcc/rust/typecheck/rust-tyty.cc | 42 +++++++++++++++-------------- 4 files changed, 39 insertions(+), 48 deletions(-) diff --git a/gcc/rust/backend/rust-compile.cc b/gcc/rust/backend/rust-compile.cc index d6c606688211..d1fd064d8950 100644 --- a/gcc/rust/backend/rust-compile.cc +++ b/gcc/rust/backend/rust-compile.cc @@ -224,12 +224,12 @@ CompileExpr::visit (HIR::MethodCallExpr &expr) std::vector 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, @@ -414,12 +414,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 diff --git a/gcc/rust/hir/tree/rust-hir-expr.h b/gcc/rust/hir/tree/rust-hir-expr.h index d66640d6f397..1d8af6beb4ef 100644 --- a/gcc/rust/hir/tree/rust-hir-expr.h +++ b/gcc/rust/hir/tree/rust-hir-expr.h @@ -1725,21 +1725,13 @@ class MethodCallExpr : public ExprWithoutBlock PathExprSegment get_method_name () const { return method_name; }; - std::vector > &get_params () { return params; } - const std::vector > &get_params () const - { - return params; - } - size_t num_params () const { return params.size (); } - void iterate_params (std::function cb) + std::vector > &get_arguments () { return params; } + + const std::vector > &get_arguments () const { - for (auto ¶m : params) - { - if (!cb (param.get ())) - return; - } + return params; } protected: diff --git a/gcc/rust/lint/rust-lint-marklive.cc b/gcc/rust/lint/rust-lint-marklive.cc index 087129131d19..4b095ab45bdf 100644 --- a/gcc/rust/lint/rust-lint-marklive.cc +++ b/gcc/rust/lint/rust-lint-marklive.cc @@ -149,10 +149,8 @@ MarkLive::visit (HIR::MethodCallExpr &expr) { expr.get_receiver ()->accept_vis (*this); visit_path_segment (expr.get_method_name ()); - expr.iterate_params ([&] (HIR::Expr *param) -> bool { - param->accept_vis (*this); - return true; - }); + for (auto &argument : expr.get_arguments ()) + argument->accept_vis (*this); // Trying to find the method definition and mark it alive. NodeId ast_node_id = expr.get_mappings ().get_nodeid (); diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index 8de9d9d42a8f..64eab3008c09 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -2561,29 +2561,31 @@ TypeCheckMethodCallExpr::visit (FnType &type) } size_t i = 1; - call.iterate_params ([&] (HIR::Expr *param) mutable -> bool { - auto fnparam = type.param_at (i); - auto argument_expr_tyty = Resolver::TypeCheckExpr::Resolve (param, false); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "failed to resolve type for argument expr in CallExpr"); - return false; - } + for (auto &argument : call.get_arguments ()) + { + auto fnparam = type.param_at (i); + auto argument_expr_tyty + = Resolver::TypeCheckExpr::Resolve (argument.get (), false); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at ( + argument->get_locus (), + "failed to resolve type for argument expr in CallExpr"); + return; + } - auto resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (param->get_locus (), - "Type Resolution failure on parameter"); - return false; - } + auto resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); + if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + { + rust_error_at (argument->get_locus (), + "Type Resolution failure on parameter"); + return; + } - context->insert_type (param->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), resolved_argument_type); - i++; - return true; - }); + i++; + } if (i != num_args_to_call) { From 591b43e42e7f63841ce46fdd4f2760e47b6a7b0d Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Tue, 5 Oct 2021 14:43:24 +0100 Subject: [PATCH 3/5] Coercion site type checking in CallExprs must hold onto the argument type Coercion sites like CallExpr arguments must coerce the arguments for type checking, but we must still insert the type of the actual argument for that mapping not the coerced type. For example we might have: ```rust fn dynamic_dispatch(t: &dyn Bar) { t.baz(); } fn main() { let a = &Foo(123); dynamic_dispatch(a); } ``` Here the argument 'a' has a type of (&ADT{Foo}) but this is coerceable to (&dyn{Bar}) which is fine. The backend needs to be able to detect the coercion from the two types in order to generate the vtable code. This patch fixes the type checking such that we store the actual type of (&ADT{Foo}) at that argument mapping instead of the coerced one. Addresses: #700 --- gcc/rust/typecheck/rust-tyty.cc | 17 ++++++++--------- gcc/testsuite/rust/compile/func3.rs | 6 +++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index 64eab3008c09..05e594298dcd 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -2454,14 +2454,13 @@ TypeCheckCallExpr::visit (FnType &type) return; } - auto resolved_argument_type = argument_expr_tyty; - // it might be a varadic function if (i < type.num_params ()) { auto fnparam = type.param_at (i); - resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + auto resolved_argument_type + = fnparam.second->coerce (argument_expr_tyty); + if (resolved_argument_type->get_kind () == TyTy::TypeKind::ERROR) { rust_error_at (argument->get_locus (), "Type Resolution failure on parameter"); @@ -2469,7 +2468,7 @@ TypeCheckCallExpr::visit (FnType &type) } } - context->insert_type (argument->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), argument_expr_tyty); i++; } @@ -2522,14 +2521,14 @@ TypeCheckCallExpr::visit (FnPtr &type) } auto resolved_argument_type = fnparam->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + if (resolved_argument_type->get_kind () == TyTy::TypeKind::ERROR) { rust_error_at (argument->get_locus (), "Type Resolution failure on parameter"); return; } - context->insert_type (argument->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), argument_expr_tyty); i++; } @@ -2575,14 +2574,14 @@ TypeCheckMethodCallExpr::visit (FnType &type) } auto resolved_argument_type = fnparam.second->coerce (argument_expr_tyty); - if (argument_expr_tyty->get_kind () == TyTy::TypeKind::ERROR) + if (resolved_argument_type->get_kind () == TyTy::TypeKind::ERROR) { rust_error_at (argument->get_locus (), "Type Resolution failure on parameter"); return; } - context->insert_type (argument->get_mappings (), resolved_argument_type); + context->insert_type (argument->get_mappings (), argument_expr_tyty); i++; } diff --git a/gcc/testsuite/rust/compile/func3.rs b/gcc/testsuite/rust/compile/func3.rs index 6cedf8e5da37..3ab374a97b43 100644 --- a/gcc/testsuite/rust/compile/func3.rs +++ b/gcc/testsuite/rust/compile/func3.rs @@ -3,5 +3,9 @@ fn test(a: i32, b: i32) -> i32 { } fn main() { - let a = test(1, true); // { dg-error "expected .i32. got .bool." } + let a = test(1, true); + // { dg-error "expected .i32. got .bool." "" { target *-*-* } .-1 } + // { dg-error "Type Resolution failure on parameter" "" { target *-*-* } .-2 } + // { dg-error "failed to lookup type to CallExpr" "" { target *-*-* } .-3 } + // { dg-error "failed to type resolve expression" "" { target *-*-* } .-4 } } From f6a04f38d51f6ca4319219f101e3f58660b128dc Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Tue, 5 Oct 2021 14:50:29 +0100 Subject: [PATCH 4/5] Ensure we emit the code for coercion sites on CallExpr and MethodCallExpr When we coerce the types of arguments to the parameters of functions for example we must store the actual type of the argument at that HIR ID not the coerced ones. This gives the backend a chance to then figure out when to actually implement any coercion site code. such as computing the dynamic objects. Fixes: #700 --- gcc/rust/backend/rust-compile-expr.h | 22 +++- gcc/rust/backend/rust-compile.cc | 111 +++++++++++++++--- .../rust/execute/torture/coercion1.rs | 43 +++++++ .../rust/execute/torture/coercion2.rs | 41 +++++++ 4 files changed, 199 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/rust/execute/torture/coercion1.rs create mode 100644 gcc/testsuite/rust/execute/torture/coercion2.rs diff --git a/gcc/rust/backend/rust-compile-expr.h b/gcc/rust/backend/rust-compile-expr.h index eb245dce5be7..b1a0f07e7ba0 100644 --- a/gcc/rust/backend/rust-compile-expr.h +++ b/gcc/rust/backend/rust-compile-expr.h @@ -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); } diff --git a/gcc/rust/backend/rust-compile.cc b/gcc/rust/backend/rust-compile.cc index d1fd064d8950..23a035fd4617 100644 --- a/gcc/rust/backend/rust-compile.cc +++ b/gcc/rust/backend/rust-compile.cc @@ -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); + 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 vals; - for (auto &argument : expr.get_arguments ()) + for (size_t i = 0; i < expr.get_arguments ().size (); i++) { - Bexpression *e = CompileExpr::Compile (argument.get (), ctx); - vals.push_back (e); + 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 (base); + *result = fn->param_at (index); + + return true; + } + + const TyTy::FnType *fn = static_cast (base); + auto param = fn->param_at (index); + *result = param.second; + + return true; + }; + + bool is_varadic = false; + if (tyty->get_kind () == TyTy::TypeKind::FNDEF) + { + const TyTy::FnType *fn = static_cast (tyty); + is_varadic = fn->is_varadic (); + } + + size_t required_num_args; + if (tyty->get_kind () == TyTy::TypeKind::FNDEF) + { + const TyTy::FnType *fn = static_cast (tyty); + required_num_args = fn->num_params (); + } + else + { + const TyTy::FnPtr *fn = static_cast (tyty); + required_num_args = fn->num_params (); + } std::vector args; - for (auto &argument : expr.get_arguments ()) + for (size_t i = 0; i < expr.get_arguments ().size (); i++) { - Bexpression *compiled_expr - = CompileExpr::Compile (argument.get (), ctx); - args.push_back (compiled_expr); + 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 ()); } } diff --git a/gcc/testsuite/rust/execute/torture/coercion1.rs b/gcc/testsuite/rust/execute/torture/coercion1.rs new file mode 100644 index 000000000000..dff98098a425 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/coercion1.rs @@ -0,0 +1,43 @@ +/* { dg-output "123\n123\n" } */ +extern "C" { + fn printf(s: *const i8, ...); +} + +struct Foo(i32); +trait Bar { + fn baz(&self); + // { dg-warning "unused name" "" { target *-*-* } .-1 } +} + +impl Bar for Foo { + fn baz(&self) { + // { dg-warning "unused name" "" { target *-*-* } .-1 } + unsafe { + let a = "%i\n\0"; + let b = a as *const str; + let c = b as *const i8; + + printf(c, self.0); + } + } +} + +fn static_dispatch(t: &T) { + t.baz(); +} + +fn dynamic_dispatch(t: &dyn Bar) { + t.baz(); +} + +fn main() -> i32 { + let a; + a = Foo(123); + static_dispatch(&a); + + let b: &dyn Bar; + b = &a; + dynamic_dispatch(b); + + 0 +} diff --git a/gcc/testsuite/rust/execute/torture/coercion2.rs b/gcc/testsuite/rust/execute/torture/coercion2.rs new file mode 100644 index 000000000000..e40495467875 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/coercion2.rs @@ -0,0 +1,41 @@ +/* { dg-output "123\n123\n" } */ +extern "C" { + fn printf(s: *const i8, ...); +} + +struct Foo(i32); +trait Bar { + fn baz(&self); + // { dg-warning "unused name" "" { target *-*-* } .-1 } +} + +impl Bar for Foo { + fn baz(&self) { + // { dg-warning "unused name" "" { target *-*-* } .-1 } + unsafe { + let a = "%i\n\0"; + let b = a as *const str; + let c = b as *const i8; + + printf(c, self.0); + } + } +} + +fn static_dispatch(t: &T) { + t.baz(); +} + +fn dynamic_dispatch(t: &dyn Bar) { + t.baz(); +} + +fn main() -> i32 { + let a; + a = &Foo(123); + + static_dispatch(a); + dynamic_dispatch(a); + + 0 +} From 85338a7f1ca9bc6d62ea3eb3e0c796b31a58bbbe Mon Sep 17 00:00:00 2001 From: David Faust Date: Thu, 7 Oct 2021 09:42:50 -0700 Subject: [PATCH 5/5] Remove lambda iterators in various HIR classes 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, #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 eb245dce5be7..7c4046680e9c 100644 --- a/gcc/rust/backend/rust-compile-expr.h +++ b/gcc/rust/backend/rust-compile-expr.h @@ -412,11 +412,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 @@ -646,11 +646,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 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 05bc1f9f0554..3fb6c879f7dd 100644 --- a/gcc/rust/hir/tree/rust-hir-expr.h +++ b/gcc/rust/hir/tree/rust-hir-expr.h @@ -796,14 +796,7 @@ class ArrayElemsValues : public ArrayElems size_t get_num_elements () const { return values.size (); } - void iterate (std::function cb) - { - for (auto it = values.begin (); it != values.end (); it++) - { - if (!cb ((*it).get ())) - return; - } - } + std::vector > &get_values () { return values; } protected: ArrayElemsValues *clone_array_elems_impl () const override @@ -1070,15 +1063,6 @@ class TupleExpr : public ExprWithoutBlock bool is_unit () const { return tuple_elems.size () == 0; } - void iterate (std::function 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 @@ class StructExprStructFields : public StructExprStruct void accept_vis (HIRVisitor &vis) override; - void iterate (std::function cb) - { - for (auto &field : fields) - { - if (!cb (field.get ())) - return; - } - } - std::vector > &get_fields () { return fields; @@ -1510,11 +1485,6 @@ class StructExprStructFields : public StructExprStruct return fields; }; - std::vector > get_fields_as_owner () - { - return std::move (fields); - }; - void set_fields_as_owner ( std::vector > new_fields) { diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h index 8cd7a01b2e8d..54e32f746549 100644 --- a/gcc/rust/hir/tree/rust-hir-item.h +++ b/gcc/rust/hir/tree/rust-hir-item.h @@ -1492,14 +1492,7 @@ class StructStruct : public Struct void accept_vis (HIRVisitor &vis) override; - void iterate (std::function cb) - { - for (auto &field : fields) - { - if (!cb (field)) - return; - } - } + std::vector &get_fields () { return fields; } protected: /* Use covariance to implement clone function as returning this object @@ -1610,15 +1603,6 @@ class TupleStruct : public Struct std::vector &get_fields () { return fields; } const std::vector &get_fields () const { return fields; } - void iterate (std::function 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 062bb96bc0f3..cd72ef0243df 100644 --- a/gcc/rust/lint/rust-lint-marklive.h +++ b/gcc/rust/lint/rust-lint-marklive.h @@ -81,18 +81,18 @@ class MarkLive : public MarkLiveBase 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 @@ class MarkLive : public MarkLiveBase 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 464852a9f8f4..152858a9e132 100644 --- a/gcc/rust/lint/rust-lint-scan-deadcode.h +++ b/gcc/rust/lint/rust-lint-scan-deadcode.h @@ -88,16 +88,16 @@ class ScanDeadcode : public MarkLiveBase 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 28b985108cf1..d9eeb4e37592 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 @@ class TypeCheckExpr : public TypeCheckBase void visit (HIR::ArrayElemsValues &elems) override { std::vector 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 5f4721b955ba..3f8d17e53072 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 @@ class TypeCheckStmt : public TypeCheckBase std::vector 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 @@ class TypeCheckStmt : public TypeCheckBase } std::vector 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 9fac813c46db..131149fabebf 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 @@ class TypeCheckTopLevel : public TypeCheckBase std::vector 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 @@ class TypeCheckTopLevel : public TypeCheckBase } std::vector 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 0cd6883958d7..a30f4c43a365 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 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 > 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 > 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 cc2e3c01110a..6856d0538b8e 100644 --- a/gcc/rust/typecheck/rust-tycheck-dump.h +++ b/gcc/rust/typecheck/rust-tycheck-dump.h @@ -162,11 +162,11 @@ class TypeResolverDump : public TypeCheckBase 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