From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id D7A6E39960D9; Wed, 8 Jun 2022 11:53:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D7A6E39960D9 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Thomas Schwinge To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] Set TREE_ADDRESSABLE when we need to borrow any expression X-Act-Checkin: gcc X-Git-Author: Philip Herron X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: a41851dfb5bec6f40fd89db01ae75fee557306ee X-Git-Newrev: 8b36f2b80ebd0e0304b6df4020b5992640662667 Message-Id: <20220608115356.D7A6E39960D9@sourceware.org> Date: Wed, 8 Jun 2022 11:53:56 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jun 2022 11:53:57 -0000 https://gcc.gnu.org/g:8b36f2b80ebd0e0304b6df4020b5992640662667 commit 8b36f2b80ebd0e0304b6df4020b5992640662667 Author: Philip Herron Date: Tue Nov 23 17:09:52 2021 +0000 Set TREE_ADDRESSABLE when we need to borrow any expression GCC requires VAR_DECL's and PARAM_DECL's to be marked with TREE_ADDRESSABLE when the declaration will be used in borrow's ('&' getting the address). This takes into account the implicit addresses when we do autoderef in method resolution/operator-overloading. This patch keeps a seperate side table for the VAR_DECL/PARAM_DECL hir-id's for lookup. The typechecker marks these id's using AddressTakenResolver::SetAddressTaken (HIR::Expr&); Its quite simple as it only cares about paths such as: - PathInExpression - QualifiedPathInExpression - IdentifierExpression The rest of the expression types will be folded into temporary values anyway so they don't need to be marked as needs_address. Fixes #804 Diff: --- gcc/rust/Make-lang.in | 1 + gcc/rust/backend/rust-compile-fnparam.h | 33 +++-- gcc/rust/backend/rust-compile-var-decl.h | 23 +-- gcc/rust/hir/tree/rust-hir-item.h | 2 +- gcc/rust/typecheck/rust-autoderef.h | 27 +++- gcc/rust/typecheck/rust-hir-address-taken.cc | 65 +++++++++ gcc/rust/typecheck/rust-hir-address-taken.h | 159 +++++++++++++++++++++ gcc/rust/typecheck/rust-hir-type-check-expr.h | 18 ++- .../rust/execute/torture/operator_overload_6.rs | 40 ++++++ 9 files changed, 344 insertions(+), 24 deletions(-) diff --git a/gcc/rust/Make-lang.in b/gcc/rust/Make-lang.in index f0e9bc33825..6aaa0fc3227 100644 --- a/gcc/rust/Make-lang.in +++ b/gcc/rust/Make-lang.in @@ -86,6 +86,7 @@ GRS_OBJS = \ rust/rust-hir-const-fold.o \ rust/rust-hir-type-check-type.o \ rust/rust-hir-type-check-struct.o \ + rust/rust-hir-address-taken.o \ rust/rust-substitution-mapper.o \ rust/rust-lint-marklive.o \ rust/rust-hir-type-check-path.o \ diff --git a/gcc/rust/backend/rust-compile-fnparam.h b/gcc/rust/backend/rust-compile-fnparam.h index 629f0f5a873..126b49f690e 100644 --- a/gcc/rust/backend/rust-compile-fnparam.h +++ b/gcc/rust/backend/rust-compile-fnparam.h @@ -20,6 +20,7 @@ #define RUST_COMPILE_FNPARAM #include "rust-compile-base.h" +#include "rust-hir-address-taken.h" namespace Rust { namespace Compile { @@ -33,9 +34,9 @@ public: HIR::FunctionParam *param, tree decl_type, Location locus) { - CompileFnParam compiler (ctx, fndecl, decl_type, locus); + CompileFnParam compiler (ctx, fndecl, decl_type, locus, *param); param->get_param_name ()->accept_vis (compiler); - return compiler.translated; + return compiler.compiled_param; } void visit (HIR::IdentifierPattern &pattern) override @@ -43,23 +44,31 @@ public: if (!pattern.is_mut ()) decl_type = ctx->get_backend ()->immutable_type (decl_type); - translated + bool address_taken = false; + address_taken_context->lookup_addess_taken ( + param.get_mappings ().get_hirid (), &address_taken); + + compiled_param = ctx->get_backend ()->parameter_variable (fndecl, pattern.variable_ident, - decl_type, - false /* address_taken */, + decl_type, address_taken, locus); } private: - CompileFnParam (Context *ctx, tree fndecl, tree decl_type, Location locus) + CompileFnParam (Context *ctx, tree fndecl, tree decl_type, Location locus, + const HIR::FunctionParam ¶m) : HIRCompileBase (ctx), fndecl (fndecl), decl_type (decl_type), - locus (locus), translated (nullptr) + locus (locus), param (param), + compiled_param (ctx->get_backend ()->error_variable ()), + address_taken_context (Resolver::AddressTakenContext::get ()) {} tree fndecl; tree decl_type; Location locus; - ::Bvariable *translated; + const HIR::FunctionParam ¶m; + Bvariable *compiled_param; + const Resolver::AddressTakenContext *address_taken_context; }; class CompileSelfParam : public HIRCompileBase @@ -74,9 +83,13 @@ public: if (is_immutable) decl_type = ctx->get_backend ()->immutable_type (decl_type); + const auto &address_taken_context = Resolver::AddressTakenContext::get (); + bool address_taken = false; + address_taken_context->lookup_addess_taken ( + self.get_mappings ().get_hirid (), &address_taken); + return ctx->get_backend ()->parameter_variable (fndecl, "self", decl_type, - false /* address_taken */, - locus); + address_taken, locus); } }; diff --git a/gcc/rust/backend/rust-compile-var-decl.h b/gcc/rust/backend/rust-compile-var-decl.h index a964fa2206b..097f5b8af38 100644 --- a/gcc/rust/backend/rust-compile-var-decl.h +++ b/gcc/rust/backend/rust-compile-var-decl.h @@ -20,6 +20,7 @@ #define RUST_COMPILE_VAR_DECL #include "rust-compile-base.h" +#include "rust-hir-address-taken.h" namespace Rust { namespace Compile { @@ -33,10 +34,9 @@ public: { CompileVarDecl compiler (ctx, fndecl); stmt->accept_vis (compiler); - rust_assert (compiler.translated != nullptr); ctx->insert_var_decl (stmt->get_mappings ().get_hirid (), - compiler.translated); - return compiler.translated; + compiler.compiled_variable); + return compiler.compiled_variable; } void visit (HIR::LetStmt &stmt) override @@ -47,6 +47,8 @@ public: &resolved_type); rust_assert (ok); + address_taken_context->lookup_addess_taken ( + stmt.get_mappings ().get_hirid (), &address_taken); translated_type = TyTyResolveCompile::compile (ctx, resolved_type); stmt.get_pattern ()->accept_vis (*this); } @@ -56,22 +58,27 @@ public: if (!pattern.is_mut ()) translated_type = ctx->get_backend ()->immutable_type (translated_type); - translated + compiled_variable = ctx->get_backend ()->local_variable (fndecl, pattern.variable_ident, translated_type, NULL /*decl_var*/, - false /*address_taken*/, locus); + address_taken, locus); } private: CompileVarDecl (Context *ctx, tree fndecl) - : HIRCompileBase (ctx), fndecl (fndecl), translated_type (nullptr), - translated (nullptr) + : HIRCompileBase (ctx), fndecl (fndecl), + translated_type (ctx->get_backend ()->error_type ()), + compiled_variable (ctx->get_backend ()->error_variable ()), + address_taken (false), + address_taken_context (Resolver::AddressTakenContext::get ()) {} tree fndecl; tree translated_type; Location locus; - ::Bvariable *translated; + Bvariable *compiled_variable; + bool address_taken; + const Resolver::AddressTakenContext *address_taken_context; }; } // namespace Compile diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h index 0dd7ac80b65..99a3c48d4bd 100644 --- a/gcc/rust/hir/tree/rust-hir-item.h +++ b/gcc/rust/hir/tree/rust-hir-item.h @@ -560,7 +560,7 @@ public: Type *get_type () { return type.get (); } - Analysis::NodeMapping &get_mappings () { return mappings; } + const Analysis::NodeMapping &get_mappings () const { return mappings; } }; // Visibility of item - if the item has it, then it is some form of public diff --git a/gcc/rust/typecheck/rust-autoderef.h b/gcc/rust/typecheck/rust-autoderef.h index db347551e84..ef3827d0995 100644 --- a/gcc/rust/typecheck/rust-autoderef.h +++ b/gcc/rust/typecheck/rust-autoderef.h @@ -73,32 +73,51 @@ class Adjuster public: Adjuster (const TyTy::BaseType *ty) : base (ty) {} - TyTy::BaseType *adjust_type (std::vector adjustments) + static bool needs_address (const std::vector &adjustments) + { + for (auto &adjustment : adjustments) + { + switch (adjustment.get_type ()) + { + case Adjustment::AdjustmentType::IMM_REF: + case Adjustment::AdjustmentType::MUT_REF: + return true; + + default: + break; + } + } + + return false; + } + + TyTy::BaseType *adjust_type (const std::vector &adjustments) { TyTy::BaseType *ty = base->clone (); for (auto &adjustment : adjustments) { switch (adjustment.get_type ()) { - case Resolver::Adjustment::AdjustmentType::IMM_REF: + case Adjustment::AdjustmentType::IMM_REF: ty = new TyTy::ReferenceType (ty->get_ref (), TyTy::TyVar (ty->get_ref ()), Mutability::Imm); break; - case Resolver::Adjustment::AdjustmentType::MUT_REF: + case Adjustment::AdjustmentType::MUT_REF: ty = new TyTy::ReferenceType (ty->get_ref (), TyTy::TyVar (ty->get_ref ()), Mutability::Mut); break; - case Resolver::Adjustment::AdjustmentType::DEREF_REF: + case Adjustment::AdjustmentType::DEREF_REF: // FIXME this really needs to support deref lang-item operator // overloads rust_assert (ty->get_kind () == TyTy::TypeKind::REF); const TyTy::ReferenceType *rr = static_cast (ty); ty = rr->get_base (); + break; } } diff --git a/gcc/rust/typecheck/rust-hir-address-taken.cc b/gcc/rust/typecheck/rust-hir-address-taken.cc new file mode 100644 index 00000000000..ae77763efb2 --- /dev/null +++ b/gcc/rust/typecheck/rust-hir-address-taken.cc @@ -0,0 +1,65 @@ +// Copyright (C) 2020-2021 Free Software Foundation, Inc. + +// This file is part of GCC. + +// GCC is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3, or (at your option) any later +// version. + +// GCC is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +// for more details. + +// You should have received a copy of the GNU General Public License +// along with GCC; see the file COPYING3. If not see +// . + +#include "rust-hir-address-taken.h" +#include "rust-hir-full.h" + +namespace Rust { +namespace Resolver { + +AddressTakenContext * +AddressTakenContext::get () +{ + static AddressTakenContext *instance; + if (instance == nullptr) + instance = new AddressTakenContext (); + + return instance; +} + +AddressTakenContext::~AddressTakenContext () {} + +bool +AddressTakenContext::lookup_addess_taken (HirId id, bool *address_taken) const +{ + const auto &it = ctx.find (id); + if (it == ctx.end ()) + return false; + + *address_taken = it->second; + return true; +} + +void +AddressTakenContext::insert_address_taken (HirId id, bool address_taken) +{ + const auto &it = ctx.find (id); + if (it != ctx.end ()) + { + // assert that we never change a true result to a negative + if (it->second == true) + { + rust_assert (address_taken != false); + } + } + + ctx[id] = address_taken; +} + +} // namespace Resolver +} // namespace Rust diff --git a/gcc/rust/typecheck/rust-hir-address-taken.h b/gcc/rust/typecheck/rust-hir-address-taken.h new file mode 100644 index 00000000000..19cbdef3cbf --- /dev/null +++ b/gcc/rust/typecheck/rust-hir-address-taken.h @@ -0,0 +1,159 @@ +// Copyright (C) 2020-2021 Free Software Foundation, Inc. + +// This file is part of GCC. + +// GCC is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3, or (at your option) any later +// version. + +// GCC is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +// for more details. + +// You should have received a copy of the GNU General Public License +// along with GCC; see the file COPYING3. If not see +// . + +#ifndef RUST_HIR_ADDRESS_TAKEN +#define RUST_HIR_ADDRESS_TAKEN + +#include "rust-hir-type-check-base.h" + +namespace Rust { +namespace Resolver { + +class AddressTakenContext +{ +public: + static AddressTakenContext *get (); + + ~AddressTakenContext (); + + bool lookup_addess_taken (HirId id, bool *address_taken) const; + + void insert_address_taken (HirId id, bool address_taken); + +private: + std::map ctx; +}; + +class AddressTakenResolver : public TypeCheckBase +{ + using Rust::Resolver::TypeCheckBase::visit; + +public: + static void SetAddressTaken (HIR::Expr &expr) + { + AddressTakenResolver resolver; + expr.accept_vis (resolver); + } + + void visit (HIR::IdentifierExpr &expr) override + { + NodeId ast_node_id = expr.get_mappings ().get_nodeid (); + NodeId ref_node_id = UNKNOWN_NODEID; + if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id)) + { + // these ref_node_ids will resolve to a pattern declaration but we are + // interested in the definition that this refers to get the parent id + Definition def; + if (!resolver->lookup_definition (ref_node_id, &def)) + { + rust_error_at (expr.get_locus (), + "unknown reference for resolved name"); + return; + } + ref_node_id = def.parent; + } + + if (ref_node_id == UNKNOWN_NODEID) + return; + + // node back to HIR + HirId ref = UNKNOWN_HIRID; + bool reverse_lookup + = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (), + ref_node_id, &ref); + rust_assert (reverse_lookup); + context->insert_address_taken (ref, true); + } + + void visit (HIR::PathInExpression &expr) override + { + NodeId ast_node_id = expr.get_mappings ().get_nodeid (); + NodeId ref_node_id = UNKNOWN_NODEID; + if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id)) + { + // these ref_node_ids will resolve to a pattern declaration but we are + // interested in the definition that this refers to get the parent id + Definition def; + if (!resolver->lookup_definition (ref_node_id, &def)) + { + rust_error_at (expr.get_locus (), + "unknown reference for resolved name"); + return; + } + ref_node_id = def.parent; + } + + if (ref_node_id == UNKNOWN_NODEID) + return; + + // node back to HIR + HirId ref = UNKNOWN_HIRID; + bool reverse_lookup + = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (), + ref_node_id, &ref); + rust_assert (reverse_lookup); + context->insert_address_taken (ref, true); + } + + void visit (HIR::QualifiedPathInExpression &expr) override + { + NodeId ast_node_id = expr.get_mappings ().get_nodeid (); + NodeId ref_node_id = UNKNOWN_NODEID; + if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id)) + { + // these ref_node_ids will resolve to a pattern declaration but we are + // interested in the definition that this refers to get the parent id + Definition def; + if (!resolver->lookup_definition (ref_node_id, &def)) + { + rust_error_at (expr.get_locus (), + "unknown reference for resolved name"); + return; + } + ref_node_id = def.parent; + } + + if (ref_node_id == UNKNOWN_NODEID) + return; + + // node back to HIR + HirId ref = UNKNOWN_HIRID; + bool reverse_lookup + = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (), + ref_node_id, &ref); + rust_assert (reverse_lookup); + context->insert_address_taken (ref, true); + } + + void visit (HIR::DereferenceExpr &expr) override + { + expr.get_expr ()->accept_vis (*this); + } + +private: + AddressTakenResolver () + : TypeCheckBase (), context (AddressTakenContext::get ()) + {} + + AddressTakenContext *context; +}; + +} // namespace Resolver +} // namespace Rust + +#endif // RUST_HIR_ADDRESS_TAKEN diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.h b/gcc/rust/typecheck/rust-hir-type-check-expr.h index 9eef755e140..fb1cd972293 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.h +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.h @@ -31,6 +31,7 @@ #include "rust-hir-trait-resolve.h" #include "rust-hir-type-bounds.h" #include "rust-hir-dot-operator.h" +#include "rust-hir-address-taken.h" namespace Rust { namespace Resolver { @@ -313,6 +314,10 @@ public: Adjuster adj (receiver_tyty); TyTy::BaseType *adjusted_self = adj.adjust_type (adjustments); + // mark the required tree addressable + if (Adjuster::needs_address (adjustments)) + AddressTakenResolver::SetAddressTaken (*expr.get_receiver ().get ()); + // store the adjustments for code-generation to know what to do context->insert_autoderef_mappings (expr.get_mappings ().get_hirid (), std::move (adjustments)); @@ -1191,11 +1196,18 @@ public: TyTy::BaseType *resolved_base = TypeCheckExpr::Resolve (expr.get_expr ().get (), false); - // FIXME double_reference + if (expr.get_is_double_borrow ()) + { + // FIXME double_reference + gcc_unreachable (); + } infered = new TyTy::ReferenceType (expr.get_mappings ().get_hirid (), TyTy::TyVar (resolved_base->get_ref ()), expr.get_mut ()); + + // mark the borrowed as address_taken + AddressTakenResolver::SetAddressTaken (*expr.get_expr ().get ()); } void visit (HIR::DereferenceExpr &expr) override @@ -1277,6 +1289,10 @@ protected: PathProbeCandidate *resolved_candidate = MethodResolution::Select (candidates, lhs, adjustments); + // mark the required tree addressable + if (Adjuster::needs_address (adjustments)) + AddressTakenResolver::SetAddressTaken (*expr.get_expr ().get ()); + // is this the case we are recursive // handle the case where we are within the impl block for this lang_item // otherwise we end up with a recursive operator overload such as the i32 diff --git a/gcc/testsuite/rust/execute/torture/operator_overload_6.rs b/gcc/testsuite/rust/execute/torture/operator_overload_6.rs new file mode 100644 index 00000000000..f6ad5c41316 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/operator_overload_6.rs @@ -0,0 +1,40 @@ +/* { dg-output "add_assign\n3\n" } */ +extern "C" { + fn printf(s: *const i8, ...); +} + +#[lang = "add_assign"] +pub trait AddAssign { + fn add_assign(&mut self, rhs: Rhs); + // { dg-warning "unused name .self." "" { target *-*-* } .-1 } + // { dg-warning "unused name .rhs." "" { target *-*-* } .-2 } + // { dg-warning "unused name .AddAssign::add_assign." "" { target *-*-* } .-3 } +} + +impl AddAssign for i32 { + fn add_assign(&mut self, other: i32) { + unsafe { + let a = "add_assign\n\0"; + let b = a as *const str; + let c = b as *const i8; + + printf(c); + } + *self += other + } +} + +fn main() -> i32 { + let mut res = 1; + res += 2; + + unsafe { + let a = "%i\n\0"; + let b = a as *const str; + let c = b as *const i8; + + printf(c, res); + } + + 0 +}