Skip to content

Commit

Permalink
Merge #814
Browse files Browse the repository at this point in the history
814: Set TREE_ADDRESSABLE when we need to borrow any expression r=philberty a=philberty

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.

If it is not set we end up in cases like this:

```c
i32 main ()
{
  i32 a.1;
  i32 D.86;
  i32 a;

  a = 1;
  a.1 = a; // this is wrong
  <i32 as AddAssign>::add_assign (&a.1, 2);
  D.86 = 0;
  return D.86;
}
```

You can see GCC will automatically make a copy of the VAR_DECL resulting bad code-generation.

Fixes #804


Co-authored-by: Philip Herron <philip.herron@embecosm.com>
  • Loading branch information
bors[bot] and philberty committed Nov 23, 2021
2 parents a41851d + 8b36f2b commit 717b6da
Show file tree
Hide file tree
Showing 9 changed files with 344 additions and 24 deletions.
1 change: 1 addition & 0 deletions gcc/rust/Make-lang.in
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
33 changes: 23 additions & 10 deletions gcc/rust/backend/rust-compile-fnparam.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define RUST_COMPILE_FNPARAM

#include "rust-compile-base.h"
#include "rust-hir-address-taken.h"

namespace Rust {
namespace Compile {
Expand All @@ -33,33 +34,41 @@ class CompileFnParam : public HIRCompileBase
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
{
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 &param)
: 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 &param;
Bvariable *compiled_param;
const Resolver::AddressTakenContext *address_taken_context;
};

class CompileSelfParam : public HIRCompileBase
Expand All @@ -74,9 +83,13 @@ class CompileSelfParam : public HIRCompileBase
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);
}
};

Expand Down
23 changes: 15 additions & 8 deletions gcc/rust/backend/rust-compile-var-decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define RUST_COMPILE_VAR_DECL

#include "rust-compile-base.h"
#include "rust-hir-address-taken.h"

namespace Rust {
namespace Compile {
Expand All @@ -33,10 +34,9 @@ class CompileVarDecl : public HIRCompileBase
{
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
Expand All @@ -47,6 +47,8 @@ class CompileVarDecl : public HIRCompileBase
&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);
}
Expand All @@ -56,22 +58,27 @@ class CompileVarDecl : public HIRCompileBase
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
Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/hir/tree/rust-hir-item.h
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ struct FunctionParam

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
Expand Down
27 changes: 23 additions & 4 deletions gcc/rust/typecheck/rust-autoderef.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,51 @@ class Adjuster
public:
Adjuster (const TyTy::BaseType *ty) : base (ty) {}

TyTy::BaseType *adjust_type (std::vector<Adjustment> adjustments)
static bool needs_address (const std::vector<Adjustment> &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<Adjustment> &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<const TyTy::ReferenceType *> (ty);
ty = rr->get_base ();

break;
}
}
Expand Down
65 changes: 65 additions & 0 deletions gcc/rust/typecheck/rust-hir-address-taken.cc
Original file line number Diff line number Diff line change
@@ -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
// <http://www.gnu.org/licenses/>.

#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

0 comments on commit 717b6da

Please sign in to comment.