Merge #540

540: Add trait-impl support for methods r=philberty a=philberty

This PR strips HIR::Method and reuses the HIR::Function with a HIR::SelfParam.
It reduces complexity in type checking and GENERIC generation.

Co-authored-by: Philip Herron <philip.herron@embecosm.com>

23 files changed, 280 insertions, 742 deletions

diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
index f9a5a6d..7865a44 100644
--- a/gcc/rust/ast/rust-item.h
+++ b/gcc/rust/ast/rust-item.h
@@ -3382,24 +3382,11 @@ class Trait : public VisItem
 {
   bool has_unsafe;
   Identifier name;
-
-  // bool has_generics;
-  // Generics generic_params;
-  std::vector<std::unique_ptr<GenericParam> > generic_params; // inlined
-
-  // bool has_type_param_bounds;
-  // TypeParamBounds type_param_bounds;
-  std::vector<std::unique_ptr<TypeParamBound> >
-    type_param_bounds; // inlined form
-
-  // bool has_where_clause;
+  std::vector<std::unique_ptr<GenericParam> > generic_params;
+  std::vector<std::unique_ptr<TypeParamBound> > type_param_bounds;
   WhereClause where_clause;
-
   std::vector<Attribute> inner_attrs;
-
-  // bool has_trait_items;
   std::vector<std::unique_ptr<TraitItem> > trait_items;
-
   Location locus;
 
 public:
@@ -3432,7 +3419,6 @@ public:
 	 std::vector<std::unique_ptr<TraitItem> > trait_items, Visibility vis,
 	 std::vector<Attribute> outer_attrs, std::vector<Attribute> inner_attrs,
 	 Location locus)
-
     : VisItem (std::move (vis), std::move (outer_attrs)),
       has_unsafe (is_unsafe), name (std::move (name)),
       generic_params (std::move (generic_params)),
@@ -3530,13 +3516,17 @@ public:
     return type_param_bounds;
   }
 
-  // TODO: is this better? Or is a "vis_block" better?
   WhereClause &get_where_clause ()
   {
     rust_assert (has_where_clause ());
     return where_clause;
   }
 
+  void insert_implict_self (std::unique_ptr<AST::GenericParam> &&param)
+  {
+    generic_params.push_back (std::move (param));
+  }
+
 protected:
   /* Use covariance to implement clone function as returning this object
    * rather than base */
diff --git a/gcc/rust/backend/rust-compile-base.h b/gcc/rust/backend/rust-compile-base.h
index c346af5..6a05c1c 100644
--- a/gcc/rust/backend/rust-compile-base.h
+++ b/gcc/rust/backend/rust-compile-base.h
@@ -121,7 +121,6 @@ public:
   // virtual void visit(WhereClauseItem& item) {}
   virtual void visit (HIR::LifetimeWhereClauseItem &item) {}
   virtual void visit (HIR::TypeBoundWhereClauseItem &item) {}
-  virtual void visit (HIR::Method &method) {}
   virtual void visit (HIR::ModuleBodied &module) {}
   virtual void visit (HIR::ModuleNoBody &module) {}
   virtual void visit (HIR::ExternCrate &crate) {}
diff --git a/gcc/rust/backend/rust-compile-implitem.h b/gcc/rust/backend/rust-compile-implitem.h
index 70f76b7..83af5de 100644
--- a/gcc/rust/backend/rust-compile-implitem.h
+++ b/gcc/rust/backend/rust-compile-implitem.h
@@ -135,212 +135,48 @@ public:
     TyTy::BaseType *tyret = fntype->get_return_type ();
     std::vector<Bvariable *> param_vars;
 
-    size_t i = 0;
-    for (auto &it : fntype->get_params ())
+    if (function.is_method ())
       {
-	HIR::FunctionParam &referenced_param
-	  = function.get_function_params ().at (i);
-	auto param_tyty = it.second;
-	auto compiled_param_type
-	  = TyTyResolveCompile::compile (ctx, param_tyty);
-
-	Location param_locus
-	  = ctx->get_mappings ()->lookup_location (param_tyty->get_ref ());
-	Bvariable *compiled_param_var
-	  = CompileFnParam::compile (ctx, fndecl, &referenced_param,
-				     compiled_param_type, param_locus);
-	if (compiled_param_var == nullptr)
+	// insert self
+	TyTy::BaseType *self_tyty_lookup = nullptr;
+	if (!ctx->get_tyctx ()->lookup_type (
+	      function.get_self_param ().get_mappings ().get_hirid (),
+	      &self_tyty_lookup))
 	  {
-	    rust_error_at (param_locus, "failed to compile parameter variable");
+	    rust_error_at (function.get_self_param ().get_locus (),
+			   "failed to lookup self param type");
 	    return;
 	  }
 
-	param_vars.push_back (compiled_param_var);
-
-	ctx->insert_var_decl (referenced_param.get_mappings ().get_hirid (),
-			      compiled_param_var);
-	i++;
-      }
-
-    if (!ctx->get_backend ()->function_set_parameters (fndecl, param_vars))
-      {
-	rust_fatal_error (function.get_locus (),
-			  "failed to setup parameter variables");
-	return;
-      }
-
-    // lookup locals
-    auto block_expr = function.get_definition ().get ();
-    auto body_mappings = block_expr->get_mappings ();
-
-    Resolver::Rib *rib = nullptr;
-    if (!ctx->get_resolver ()->find_name_rib (body_mappings.get_nodeid (),
-					      &rib))
-      {
-	rust_fatal_error (function.get_locus (),
-			  "failed to setup locals per block");
-	return;
-      }
-
-    std::vector<Bvariable *> locals;
-    bool ok = compile_locals_for_block (*rib, fndecl, locals);
-    rust_assert (ok);
-
-    Bblock *enclosing_scope = NULL;
-    HIR::BlockExpr *function_body = function.get_definition ().get ();
-    Location start_location = function_body->get_locus ();
-    Location end_location = function_body->get_closing_locus ();
-
-    Bblock *code_block
-      = ctx->get_backend ()->block (fndecl, enclosing_scope, locals,
-				    start_location, end_location);
-    ctx->push_block (code_block);
-
-    Bvariable *return_address = nullptr;
-    if (function.has_function_return_type ())
-      {
-	Btype *return_type = TyTyResolveCompile::compile (ctx, tyret);
-
-	bool address_is_taken = false;
-	Bstatement *ret_var_stmt = nullptr;
-
-	return_address = ctx->get_backend ()->temporary_variable (
-	  fndecl, code_block, return_type, NULL, address_is_taken,
-	  function.get_locus (), &ret_var_stmt);
-
-	ctx->add_statement (ret_var_stmt);
-      }
-
-    ctx->push_fn (fndecl, return_address);
-
-    compile_function_body (fndecl, function.get_definition (),
-			   function.has_function_return_type ());
-
-    ctx->pop_block ();
-    auto body = ctx->get_backend ()->block_statement (code_block);
-    if (!ctx->get_backend ()->function_set_body (fndecl, body))
-      {
-	rust_error_at (function.get_locus (), "failed to set body to function");
-	return;
-      }
-
-    ctx->pop_fn ();
-
-    ctx->push_function (fndecl);
-  }
-
-  void visit (HIR::Method &method) override
-  {
-    if (!compile_fns)
-      return;
-
-    TyTy::BaseType *fntype_tyty;
-    if (!ctx->get_tyctx ()->lookup_type (method.get_mappings ().get_hirid (),
-					 &fntype_tyty))
-      {
-	rust_fatal_error (method.get_locus (),
-			  "failed to lookup function type");
-	return;
-      }
-
-    rust_assert (fntype_tyty->get_kind () == TyTy::TypeKind::FNDEF);
-    TyTy::FnType *fntype = static_cast<TyTy::FnType *> (fntype_tyty);
-    if (fntype->has_subsititions_defined ())
-      {
-	// we cant do anything for this only when it is used and a concrete type
-	// is given
-	if (concrete == nullptr)
-	  return;
-	else
+	Btype *self_type = TyTyResolveCompile::compile (ctx, self_tyty_lookup);
+	if (self_type == nullptr)
 	  {
-	    rust_assert (concrete->get_kind () == TyTy::TypeKind::FNDEF);
-	    fntype = static_cast<TyTy::FnType *> (concrete);
+	    rust_error_at (function.get_self_param ().get_locus (),
+			   "failed to compile self param type");
+	    return;
 	  }
-      }
 
-    // items can be forward compiled which means we may not need to invoke this
-    // code. We might also have already compiled this generic function as well.
-    Bfunction *lookup = nullptr;
-    if (ctx->lookup_function_decl (fntype->get_ty_ref (), &lookup, fntype))
-      {
-	// has this been added to the list then it must be finished
-	if (ctx->function_completed (lookup))
+	Bvariable *compiled_self_param
+	  = CompileSelfParam::compile (ctx, fndecl, function.get_self_param (),
+				       self_type,
+				       function.get_self_param ().get_locus ());
+	if (compiled_self_param == nullptr)
 	  {
-	    Bfunction *dummy = nullptr;
-	    if (!ctx->lookup_function_decl (fntype->get_ty_ref (), &dummy))
-	      ctx->insert_function_decl (fntype->get_ty_ref (), lookup, fntype);
-
+	    rust_error_at (function.get_self_param ().get_locus (),
+			   "failed to compile self param variable");
 	    return;
 	  }
-      }
-
-    if (fntype->has_subsititions_defined ())
-      {
-	// override the Hir Lookups for the substituions in this context
-	fntype->override_context ();
-      }
-
-    // convert to the actual function type
-    ::Btype *compiled_fn_type = TyTyResolveCompile::compile (ctx, fntype);
-
-    unsigned int flags = 0;
-
-    // if its the main fn or pub visibility mark its as DECL_PUBLIC
-    // please see https://github.com/Rust-GCC/gccrs/pull/137
-    if (method.has_visibility ())
-      flags |= Backend::function_is_visible;
-
-    std::string fn_identifier
-      = self->get_name () + "_" + method.get_method_name ();
-    std::string asm_name
-      = ctx->mangle_impl_item (self, fntype, method.get_method_name ());
-
-    Bfunction *fndecl
-      = ctx->get_backend ()->function (compiled_fn_type, fn_identifier,
-				       asm_name, flags, method.get_locus ());
-    ctx->insert_function_decl (fntype->get_ty_ref (), fndecl, fntype);
-
-    // setup the params
-    TyTy::BaseType *tyret = fntype->get_return_type ();
-    std::vector<Bvariable *> param_vars;
 
-    // insert self
-    TyTy::BaseType *self_tyty_lookup = nullptr;
-    if (!ctx->get_tyctx ()->lookup_type (
-	  method.get_self_param ().get_mappings ().get_hirid (),
-	  &self_tyty_lookup))
-      {
-	rust_error_at (method.get_self_param ().get_locus (),
-		       "failed to lookup self param type");
-	return;
-      }
-
-    Btype *self_type = TyTyResolveCompile::compile (ctx, self_tyty_lookup);
-    if (self_type == nullptr)
-      {
-	rust_error_at (method.get_self_param ().get_locus (),
-		       "failed to compile self param type");
-	return;
-      }
-
-    Bvariable *compiled_self_param
-      = CompileSelfParam::compile (ctx, fndecl, method.get_self_param (),
-				   self_type,
-				   method.get_self_param ().get_locus ());
-    if (compiled_self_param == nullptr)
-      {
-	rust_error_at (method.get_self_param ().get_locus (),
-		       "failed to compile self param variable");
-	return;
+	param_vars.push_back (compiled_self_param);
+	ctx->insert_var_decl (
+	  function.get_self_param ().get_mappings ().get_hirid (),
+	  compiled_self_param);
       }
 
-    param_vars.push_back (compiled_self_param);
-    ctx->insert_var_decl (method.get_self_param ().get_mappings ().get_hirid (),
-			  compiled_self_param);
-
-    // offset from + 1 for the TyTy::FnType being used
-    size_t i = 1;
-    for (auto referenced_param : method.get_function_params ())
+    // offset from + 1 for the TyTy::FnType being used when this is a method to
+    // skip over Self on the FnType
+    size_t i = function.is_method () ? 1 : 0;
+    for (auto referenced_param : function.get_function_params ())
       {
 	auto tyty_param = fntype->param_at (i);
 	auto param_tyty = tyty_param.second;
@@ -361,7 +197,7 @@ public:
 				     compiled_param_type, param_locus);
 	if (compiled_param_var == nullptr)
 	  {
-	    rust_error_at (param_locus, "failed to compile parameter variable");
+	    rust_error_at (param_locus, "Failed to compile parameter variable");
 	    return;
 	  }
 
@@ -374,20 +210,20 @@ public:
 
     if (!ctx->get_backend ()->function_set_parameters (fndecl, param_vars))
       {
-	rust_fatal_error (method.get_locus (),
+	rust_fatal_error (function.get_locus (),
 			  "failed to setup parameter variables");
 	return;
       }
 
     // lookup locals
-    auto block_expr = method.get_function_body ().get ();
+    auto block_expr = function.get_definition ().get ();
     auto body_mappings = block_expr->get_mappings ();
 
     Resolver::Rib *rib = nullptr;
     if (!ctx->get_resolver ()->find_name_rib (body_mappings.get_nodeid (),
 					      &rib))
       {
-	rust_fatal_error (method.get_locus (),
+	rust_fatal_error (function.get_locus (),
 			  "failed to setup locals per block");
 	return;
       }
@@ -397,7 +233,7 @@ public:
     rust_assert (ok);
 
     Bblock *enclosing_scope = NULL;
-    HIR::BlockExpr *function_body = method.get_function_body ().get ();
+    HIR::BlockExpr *function_body = function.get_definition ().get ();
     Location start_location = function_body->get_locus ();
     Location end_location = function_body->get_closing_locus ();
 
@@ -407,7 +243,7 @@ public:
     ctx->push_block (code_block);
 
     Bvariable *return_address = nullptr;
-    if (method.has_function_return_type ())
+    if (function.has_function_return_type ())
       {
 	Btype *return_type = TyTyResolveCompile::compile (ctx, tyret);
 
@@ -416,21 +252,21 @@ public:
 
 	return_address = ctx->get_backend ()->temporary_variable (
 	  fndecl, code_block, return_type, NULL, address_is_taken,
-	  method.get_locus (), &ret_var_stmt);
+	  function.get_locus (), &ret_var_stmt);
 
 	ctx->add_statement (ret_var_stmt);
       }
 
     ctx->push_fn (fndecl, return_address);
 
-    compile_function_body (fndecl, method.get_function_body (),
-			   method.has_function_return_type ());
+    compile_function_body (fndecl, function.get_definition (),
+			   function.has_function_return_type ());
 
     ctx->pop_block ();
     auto body = ctx->get_backend ()->block_statement (code_block);
     if (!ctx->get_backend ()->function_set_body (fndecl, body))
       {
-	rust_error_at (method.get_locus (), "failed to set body to function");
+	rust_error_at (function.get_locus (), "failed to set body to function");
 	return;
       }
 
diff --git a/gcc/rust/hir/rust-ast-lower-implitem.h b/gcc/rust/hir/rust-ast-lower-implitem.h
index 2e7d4d8..16c5aa0 100644
--- a/gcc/rust/hir/rust-ast-lower-implitem.h
+++ b/gcc/rust/hir/rust-ast-lower-implitem.h
@@ -142,7 +142,8 @@ public:
 			   std::move (qualifiers), std::move (generic_params),
 			   std::move (function_params), std::move (return_type),
 			   std::move (where_clause), std::move (function_body),
-			   std::move (vis), function.get_outer_attrs (), locus);
+			   std::move (vis), function.get_outer_attrs (),
+			   HIR::SelfParam::error (), locus);
 
     mappings->insert_hir_implitem (mapping.get_crate_num (),
 				   mapping.get_hirid (), parent_impl_id, fn);
@@ -217,12 +218,12 @@ public:
 				   mappings->get_next_hir_id (crate_num),
 				   mappings->get_next_localdef_id (crate_num));
     auto mth
-      = new HIR::Method (mapping, std::move (method_name),
-			 std::move (qualifiers), std::move (generic_params),
-			 std::move (self_param), std::move (function_params),
-			 std::move (return_type), std::move (where_clause),
-			 std::move (method_body), std::move (vis),
-			 method.get_outer_attrs (), locus);
+      = new HIR::Function (mapping, std::move (method_name),
+			   std::move (qualifiers), std::move (generic_params),
+			   std::move (function_params), std::move (return_type),
+			   std::move (where_clause), std::move (method_body),
+			   std::move (vis), method.get_outer_attrs (),
+			   std::move (self_param), locus);
 
     mappings->insert_hir_implitem (mapping.get_crate_num (),
 				   mapping.get_hirid (), parent_impl_id, mth);
diff --git a/gcc/rust/hir/rust-ast-lower-item.h b/gcc/rust/hir/rust-ast-lower-item.h
index bb9f764..b2a4d5f 100644
--- a/gcc/rust/hir/rust-ast-lower-item.h
+++ b/gcc/rust/hir/rust-ast-lower-item.h
@@ -308,7 +308,8 @@ public:
 			   std::move (qualifiers), std::move (generic_params),
 			   std::move (function_params), std::move (return_type),
 			   std::move (where_clause), std::move (function_body),
-			   std::move (vis), function.get_outer_attrs (), locus);
+			   std::move (vis), function.get_outer_attrs (),
+			   HIR::SelfParam::error (), locus);
 
     mappings->insert_defid_mapping (mapping.get_defid (), translated);
     mappings->insert_hir_item (mapping.get_crate_num (), mapping.get_hirid (),
diff --git a/gcc/rust/hir/rust-ast-lower-stmt.h b/gcc/rust/hir/rust-ast-lower-stmt.h
index 1dd8a10..771c3ad 100644
--- a/gcc/rust/hir/rust-ast-lower-stmt.h
+++ b/gcc/rust/hir/rust-ast-lower-stmt.h
@@ -294,7 +294,8 @@ public:
 			   std::move (qualifiers), std::move (generic_params),
 			   std::move (function_params), std::move (return_type),
 			   std::move (where_clause), std::move (function_body),
-			   std::move (vis), function.get_outer_attrs (), locus);
+			   std::move (vis), function.get_outer_attrs (),
+			   HIR::SelfParam::error (), locus);
 
     mappings->insert_hir_item (mapping.get_crate_num (), mapping.get_hirid (),
 			       fn);
diff --git a/gcc/rust/hir/tree/rust-hir-full-decls.h b/gcc/rust/hir/tree/rust-hir-full-decls.h
index ebe4bb3..58a110e 100644
--- a/gcc/rust/hir/tree/rust-hir-full-decls.h
+++ b/gcc/rust/hir/tree/rust-hir-full-decls.h
@@ -160,7 +160,6 @@ struct SelfParam;
 struct FunctionQualifiers;
 struct FunctionParam;
 struct Visibility;
-class Method;
 class VisItem;
 class Module;
 class ModuleBodied;
diff --git a/gcc/rust/hir/tree/rust-hir-full-test.cc b/gcc/rust/hir/tree/rust-hir-full-test.cc
index 236ff3d..261b3af 100644
--- a/gcc/rust/hir/tree/rust-hir-full-test.cc
+++ b/gcc/rust/hir/tree/rust-hir-full-test.cc
@@ -451,79 +451,6 @@ ImplBlock::as_string () const
 }
 
 std::string
-Method::as_string () const
-{
-  std::string str ("Method: \n ");
-
-  str += vis.as_string () + " " + qualifiers.as_string ();
-
-  str += " fn " + method_name;
-
-  // generic params
-  str += "\n Generic params: ";
-  if (generic_params.empty ())
-    {
-      str += "none";
-    }
-  else
-    {
-      for (const auto &param : generic_params)
-	{
-	  // DEBUG: null pointer check
-	  if (param == nullptr)
-	    {
-	      rust_debug (
-		"something really terrible has gone wrong - null pointer "
-		"generic param in method.");
-	      return "nullptr_POINTER_MARK";
-	    }
-
-	  str += "\n  " + param->as_string ();
-	}
-    }
-
-  str += "\n Self param: " + self_param.as_string ();
-
-  str += "\n Function params: ";
-  if (function_params.empty ())
-    {
-      str += "none";
-    }
-  else
-    {
-      for (const auto &param : function_params)
-	{
-	  str += "\n  " + param.as_string ();
-	}
-    }
-
-  str += "\n Return type: ";
-  if (has_return_type ())
-    {
-      str += return_type->as_string ();
-    }
-  else
-    {
-      str += "none (void)";
-    }
-
-  str += "\n Where clause: ";
-  if (has_where_clause ())
-    {
-      str += where_clause.as_string ();
-    }
-  else
-    {
-      str += "none";
-    }
-
-  str += "\n Block expr (body): \n  ";
-  str += function_body->as_string ();
-
-  return str;
-}
-
-std::string
 StructStruct::as_string () const
 {
   std::string str = VisItem::as_string ();
@@ -4361,12 +4288,6 @@ TypeBoundWhereClauseItem::accept_vis (HIRVisitor &vis)
 }
 
 void
-Method::accept_vis (HIRVisitor &vis)
-{
-  vis.visit (*this);
-}
-
-void
 ModuleBodied::accept_vis (HIRVisitor &vis)
 {
   vis.visit (*this);
diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
index 0cf71e7..a5069b7 100644
--- a/gcc/rust/hir/tree/rust-hir-item.h
+++ b/gcc/rust/hir/tree/rust-hir-item.h
@@ -385,7 +385,7 @@ public:
   bool has_lifetime () const { return !lifetime.is_error (); }
 
   // Returns whether the self-param is in an error state.
-  bool is_error () const { return self_kind != ImplicitSelfKind::NONE; }
+  bool is_error () const { return self_kind == ImplicitSelfKind::NONE; }
 
   std::string as_string () const;
 
@@ -592,192 +592,6 @@ protected:
   }
 };
 
-// A method (function belonging to a type)
-class Method : public ImplItem
-{
-  Analysis::NodeMapping mappings;
-
-  // moved from impl items for consistency
-  AST::AttrVec outer_attrs;
-  Visibility vis;
-
-  FunctionQualifiers qualifiers;
-  Identifier method_name;
-
-  // bool has_generics;
-  // Generics generic_params;
-  std::vector<std::unique_ptr<GenericParam> > generic_params; // inlined
-
-  SelfParam self_param;
-
-  // bool has_params;
-  // FunctionParams function_params;
-  std::vector<FunctionParam> function_params; // inlined
-
-  // bool has_return_type;
-  // FunctionReturnType return_type;
-  std::unique_ptr<Type> return_type; // inlined
-
-  // bool has_where_clause;
-  WhereClause where_clause;
-
-  std::unique_ptr<BlockExpr> function_body;
-
-  Location locus;
-
-public:
-  // Returns whether the method has generic parameters.
-  bool has_generics () const { return !generic_params.empty (); }
-
-  // Returns whether the method has parameters.
-  bool has_params () const { return !function_params.empty (); }
-
-  // Returns whether the method has a return type (void otherwise).
-  bool has_return_type () const { return return_type != nullptr; }
-
-  // Returns whether the where clause exists (i.e. has items)
-  bool has_where_clause () const { return !where_clause.is_empty (); }
-
-  // Returns whether method has a non-default visibility.
-  bool has_visibility () const { return !vis.is_error (); }
-
-  // Mega-constructor with all possible fields
-  Method (Analysis::NodeMapping mappings, Identifier method_name,
-	  FunctionQualifiers qualifiers,
-	  std::vector<std::unique_ptr<GenericParam> > generic_params,
-	  SelfParam self_param, std::vector<FunctionParam> function_params,
-	  std::unique_ptr<Type> return_type, WhereClause where_clause,
-	  std::unique_ptr<BlockExpr> function_body, Visibility vis,
-	  AST::AttrVec outer_attrs, Location locus = Location ())
-    : mappings (mappings), outer_attrs (std::move (outer_attrs)),
-      vis (std::move (vis)), qualifiers (std::move (qualifiers)),
-      method_name (std::move (method_name)),
-      generic_params (std::move (generic_params)),
-      self_param (std::move (self_param)),
-      function_params (std::move (function_params)),
-      return_type (std::move (return_type)),
-      where_clause (std::move (where_clause)),
-      function_body (std::move (function_body)), locus (locus)
-  {}
-
-  // TODO: add constructor with less fields
-
-  // Copy constructor with clone
-  Method (Method const &other)
-    : mappings (other.mappings), outer_attrs (other.outer_attrs),
-      vis (other.vis), qualifiers (other.qualifiers),
-      method_name (other.method_name), self_param (other.self_param),
-      function_params (other.function_params),
-      return_type (other.return_type->clone_type ()),
-      where_clause (other.where_clause),
-      function_body (other.function_body->clone_block_expr ()),
-      locus (other.locus)
-  {
-    generic_params.reserve (other.generic_params.size ());
-    for (const auto &e : other.generic_params)
-      generic_params.push_back (e->clone_generic_param ());
-  }
-
-  // Overloaded assignment operator to clone
-  Method &operator= (Method const &other)
-  {
-    mappings = other.mappings;
-    method_name = other.method_name;
-    outer_attrs = other.outer_attrs;
-    vis = other.vis;
-    qualifiers = other.qualifiers;
-    self_param = other.self_param;
-    function_params = other.function_params;
-    return_type = other.return_type->clone_type ();
-    where_clause = other.where_clause;
-    function_body = other.function_body->clone_block_expr ();
-    locus = other.locus;
-
-    generic_params.reserve (other.generic_params.size ());
-    for (const auto &e : other.generic_params)
-      generic_params.push_back (e->clone_generic_param ());
-
-    return *this;
-  }
-
-  // move constructors
-  Method (Method &&other) = default;
-  Method &operator= (Method &&other) = default;
-
-  std::string as_string () const override;
-
-  void accept_vis (HIRVisitor &vis) override;
-
-  Analysis::NodeMapping get_mappings () const { return mappings; };
-
-  Analysis::NodeMapping get_impl_mappings () const override
-  {
-    return get_mappings ();
-  };
-
-  // Returns whether function has return type - if not, it is void.
-  bool has_function_return_type () const { return return_type != nullptr; }
-
-  std::vector<FunctionParam> &get_function_params () { return function_params; }
-  const std::vector<FunctionParam> &get_function_params () const
-  {
-    return function_params;
-  }
-
-  std::vector<std::unique_ptr<GenericParam> > &get_generic_params ()
-  {
-    return generic_params;
-  }
-  const std::vector<std::unique_ptr<GenericParam> > &get_generic_params () const
-  {
-    return generic_params;
-  }
-
-  // TODO: is this better? Or is a "vis_block" better?
-  std::unique_ptr<BlockExpr> &get_definition ()
-  {
-    rust_assert (function_body != nullptr);
-    return function_body;
-  }
-
-  SelfParam &get_self_param () { return self_param; }
-  const SelfParam &get_self_param () const { return self_param; }
-
-  // TODO: is this better? Or is a "vis_block" better?
-  std::unique_ptr<Type> &get_return_type ()
-  {
-    rust_assert (has_return_type ());
-    return return_type;
-  }
-
-  // TODO: is this better? Or is a "vis_block" better?
-  WhereClause &get_where_clause ()
-  {
-    rust_assert (has_where_clause ());
-    return where_clause;
-  }
-
-  Identifier get_method_name () const { return method_name; }
-
-  Location get_locus () const { return locus; }
-
-  Location get_impl_locus () const final { return get_locus (); }
-
-  std::unique_ptr<BlockExpr> &get_function_body () { return function_body; }
-  const std::unique_ptr<BlockExpr> &get_function_body () const
-  {
-    return function_body;
-  }
-
-protected:
-  /* Use covariance to implement clone function as returning this object
-   * rather than base */
-  Method *clone_inherent_impl_item_impl () const override
-  {
-    return new Method (*this);
-  }
-};
-
 // Item that supports visibility - abstract base class
 class VisItem : public Item
 {
@@ -1267,11 +1081,6 @@ protected:
   }*/
 };
 
-// Parameters used in a function - TODO inline?
-/*struct FunctionParams {
-    std::vector<FunctionParam> function_params;
-};*/
-
 class LetStmt;
 
 // Rust function declaration HIR node
@@ -1279,23 +1088,12 @@ class Function : public VisItem, public ImplItem
 {
   FunctionQualifiers qualifiers;
   Identifier function_name;
-
-  // bool has_generics;
-  // Generics generic_params;
-  std::vector<std::unique_ptr<GenericParam> > generic_params; // inlined
-
-  // bool has_function_params;
-  // FunctionParams function_params;
-  std::vector<FunctionParam> function_params; // inlined
-
-  // bool has_function_return_type;
+  std::vector<std::unique_ptr<GenericParam> > generic_params;
+  std::vector<FunctionParam> function_params;
   std::unique_ptr<Type> return_type;
-
-  // bool has_where_clause;
   WhereClause where_clause;
-
   std::unique_ptr<BlockExpr> function_body;
-
+  SelfParam self;
   Location locus;
 
 public:
@@ -1320,7 +1118,7 @@ public:
 	    std::vector<FunctionParam> function_params,
 	    std::unique_ptr<Type> return_type, WhereClause where_clause,
 	    std::unique_ptr<BlockExpr> function_body, Visibility vis,
-	    AST::AttrVec outer_attrs, Location locus)
+	    AST::AttrVec outer_attrs, SelfParam self, Location locus)
     : VisItem (std::move (mappings), std::move (vis), std::move (outer_attrs)),
       qualifiers (std::move (qualifiers)),
       function_name (std::move (function_name)),
@@ -1328,11 +1126,10 @@ public:
       function_params (std::move (function_params)),
       return_type (std::move (return_type)),
       where_clause (std::move (where_clause)),
-      function_body (std::move (function_body)), locus (locus)
+      function_body (std::move (function_body)), self (std::move (self)),
+      locus (locus)
   {}
 
-  // TODO: add constructor with less fields
-
   // Copy constructor with clone
   Function (Function const &other)
     : VisItem (other), qualifiers (other.qualifiers),
@@ -1341,7 +1138,7 @@ public:
       return_type (other.return_type->clone_type ()),
       where_clause (other.where_clause),
       function_body (other.function_body->clone_block_expr ()),
-      locus (other.locus)
+      self (other.self), locus (other.locus)
   {
     generic_params.reserve (other.generic_params.size ());
     for (const auto &e : other.generic_params)
@@ -1354,14 +1151,12 @@ public:
     VisItem::operator= (other);
     function_name = other.function_name;
     qualifiers = other.qualifiers;
-    // generic_params = other.generic_params;
     function_params = other.function_params;
     return_type = other.return_type->clone_type ();
     where_clause = other.where_clause;
     function_body = other.function_body->clone_block_expr ();
-    // visibility = other.visibility->clone_visibility();
-    // outer_attrs = other.outer_attrs;
     locus = other.locus;
+    self = other.self;
 
     generic_params.reserve (other.generic_params.size ());
     for (const auto &e : other.generic_params)
@@ -1427,6 +1222,14 @@ public:
     return return_type;
   }
 
+  bool is_method () const { return !self.is_error (); }
+
+  SelfParam &get_self_param ()
+  {
+    rust_assert (is_method ());
+    return self;
+  }
+
 protected:
   /* Use covariance to implement clone function as returning this object
    * rather than base */
@@ -2364,7 +2167,7 @@ public:
       generic_params (std::move (generic_params)),
       function_params (std::move (function_params)),
       return_type (std::move (return_type)),
-      where_clause (std::move (where_clause)), self (self)
+      where_clause (std::move (where_clause)), self (std::move (self))
   {}
 
   // Copy constructor with clone
@@ -2661,24 +2464,11 @@ protected:
 class Trait : public VisItem
 {
   bool has_unsafe;
-
   Identifier name;
-
-  // bool has_generics;
-  // Generics generic_params;
-  std::vector<std::unique_ptr<GenericParam> > generic_params; // inlined
-
-  // bool has_type_param_bounds;
-  // TypeParamBounds type_param_bounds;
-  std::vector<std::unique_ptr<TypeParamBound> >
-    type_param_bounds; // inlined form
-
-  // bool has_where_clause;
+  std::vector<std::unique_ptr<GenericParam> > generic_params;
+  std::vector<std::unique_ptr<TypeParamBound> > type_param_bounds;
   WhereClause where_clause;
-
-  // bool has_trait_items;
   std::vector<std::unique_ptr<TraitItem> > trait_items;
-
   Location locus;
 
 public:
@@ -2768,6 +2558,16 @@ public:
 
   void accept_vis (HIRVisitor &vis) override;
 
+  std::vector<std::unique_ptr<GenericParam> > &get_generic_params ()
+  {
+    return generic_params;
+  }
+
+  const std::vector<std::unique_ptr<GenericParam> > &get_generic_params () const
+  {
+    return generic_params;
+  }
+
 protected:
   /* Use covariance to implement clone function as returning this object
    * rather than base */
diff --git a/gcc/rust/hir/tree/rust-hir-visitor.h b/gcc/rust/hir/tree/rust-hir-visitor.h
index 6b575de..8204a41 100644
--- a/gcc/rust/hir/tree/rust-hir-visitor.h
+++ b/gcc/rust/hir/tree/rust-hir-visitor.h
@@ -102,7 +102,6 @@ public:
   virtual void visit (TypeParam &param) = 0;
   virtual void visit (LifetimeWhereClauseItem &item) = 0;
   virtual void visit (TypeBoundWhereClauseItem &item) = 0;
-  virtual void visit (Method &method) = 0;
   virtual void visit (ModuleBodied &module) = 0;
   virtual void visit (ModuleNoBody &module) = 0;
   virtual void visit (ExternCrate &crate) = 0;
diff --git a/gcc/rust/lint/rust-lint-marklive-base.h b/gcc/rust/lint/rust-lint-marklive-base.h
index baae9ae..661dfe9 100644
--- a/gcc/rust/lint/rust-lint-marklive-base.h
+++ b/gcc/rust/lint/rust-lint-marklive-base.h
@@ -117,7 +117,6 @@ public:
 
   virtual void visit (HIR::LifetimeWhereClauseItem &) override {}
   virtual void visit (HIR::TypeBoundWhereClauseItem &) override {}
-  virtual void visit (HIR::Method &) override {}
   virtual void visit (HIR::ModuleBodied &) override {}
   virtual void visit (HIR::ModuleNoBody &) override {}
   virtual void visit (HIR::ExternCrate &) override {}
diff --git a/gcc/rust/lint/rust-lint-marklive.h b/gcc/rust/lint/rust-lint-marklive.h
index 673639a..2db5294 100644
--- a/gcc/rust/lint/rust-lint-marklive.h
+++ b/gcc/rust/lint/rust-lint-marklive.h
@@ -175,11 +175,6 @@ public:
     expr.visit_rhs (*this);
   }
 
-  void visit (HIR::Method &method) override
-  {
-    method.get_definition ().get ()->accept_vis (*this);
-  }
-
   void visit (HIR::TraitItemFunc &item) override
   {
     item.get_block_expr ()->accept_vis (*this);
diff --git a/gcc/rust/resolve/rust-ast-resolve-item.h b/gcc/rust/resolve/rust-ast-resolve-item.h
index 3800350..fafd27e 100644
--- a/gcc/rust/resolve/rust-ast-resolve-item.h
+++ b/gcc/rust/resolve/rust-ast-resolve-item.h
@@ -310,16 +310,15 @@ public:
     NodeId scope_node_id = trait.get_node_id ();
     resolver->get_type_scope ().push (scope_node_id);
 
-    // TODO
     // we need to inject an implicit self TypeParam here
-    // see: https://doc.rust-lang.org/reference/items/traits.html
+    AST::TypeParam *implicit_self
+      = new AST::TypeParam ("Self", trait.get_locus ());
+    trait.insert_implict_self (
+      std::unique_ptr<AST::GenericParam> (implicit_self));
 
-    if (trait.has_generics ())
+    for (auto &generic : trait.get_generic_params ())
       {
-	for (auto &generic : trait.get_generic_params ())
-	  {
-	    ResolveGenericParam::go (generic.get (), trait.get_node_id ());
-	  }
+	ResolveGenericParam::go (generic.get (), trait.get_node_id ());
       }
 
     for (auto &item : trait.get_trait_items ())
@@ -371,21 +370,74 @@ public:
     resolver->get_label_scope ().pop ();
   }
 
-  void visit (AST::TraitItemMethod &) override
+  void visit (AST::TraitItemMethod &func) override
   {
-    // TODO
-  }
+    NodeId scope_node_id = func.get_node_id ();
+    resolver->get_name_scope ().push (scope_node_id);
+    resolver->get_type_scope ().push (scope_node_id);
+    resolver->get_label_scope ().push (scope_node_id);
+    resolver->push_new_name_rib (resolver->get_name_scope ().peek ());
+    resolver->push_new_type_rib (resolver->get_type_scope ().peek ());
+    resolver->push_new_label_rib (resolver->get_type_scope ().peek ());
 
-  void visit (AST::TraitItemConst &) override
-  {
-    // TODO
-  }
+    AST::TraitMethodDecl &function = func.get_trait_method_decl ();
+    if (function.has_generics ())
+      {
+	for (auto &generic : function.get_generic_params ())
+	  ResolveGenericParam::go (generic.get (), func.get_node_id ());
+      }
 
-  void visit (AST::TraitItemType &) override
-  {
-    // TODO
+    if (function.has_return_type ())
+      ResolveType::go (function.get_return_type ().get (), func.get_node_id ());
+
+    // self turns into (self: Self) as a function param
+    AST::SelfParam &self_param = function.get_self_param ();
+    AST::IdentifierPattern self_pattern (
+      self_param.get_node_id (), "self", self_param.get_locus (),
+      self_param.get_has_ref (), self_param.get_is_mut (),
+      std::unique_ptr<AST::Pattern> (nullptr));
+
+    std::vector<std::unique_ptr<AST::TypePathSegment> > segments;
+    segments.push_back (std::unique_ptr<AST::TypePathSegment> (
+      new AST::TypePathSegment ("Self", false, self_param.get_locus ())));
+
+    AST::TypePath self_type_path (std::move (segments),
+				  self_param.get_locus ());
+
+    ResolveType::go (&self_type_path, self_param.get_node_id ());
+    PatternDeclaration::go (&self_pattern, self_param.get_node_id ());
+
+    resolver->mark_assignment_to_decl (self_pattern.get_node_id (),
+				       self_pattern.get_node_id ());
+
+    // we make a new scope so the names of parameters are resolved and shadowed
+    // correctly
+    for (auto &param : function.get_function_params ())
+      {
+	ResolveType::go (param.get_type ().get (), param.get_node_id ());
+	PatternDeclaration::go (param.get_pattern ().get (),
+				param.get_node_id ());
+
+	// the mutability checker needs to verify for immutable decls the number
+	// of assignments are <1. This marks an implicit assignment
+	resolver->mark_assignment_to_decl (param.get_pattern ()->get_node_id (),
+					   param.get_node_id ());
+      }
+
+    // trait items have an optional body
+    if (func.has_definition ())
+      ResolveExpr::go (func.get_definition ().get (), func.get_node_id ());
+
+    resolver->get_name_scope ().pop ();
+    resolver->get_type_scope ().pop ();
+    resolver->get_label_scope ().pop ();
   }
 
+  // TODO
+  void visit (AST::TraitItemConst &) override { gcc_unreachable (); }
+
+  void visit (AST::TraitItemType &) override { gcc_unreachable (); }
+
 private:
   ResolveItem () : ResolverBase (UNKNOWN_NODEID) {}
 };
diff --git a/gcc/rust/resolve/rust-ast-resolve-type.h b/gcc/rust/resolve/rust-ast-resolve-type.h
index bd990f7..19e7324 100644
--- a/gcc/rust/resolve/rust-ast-resolve-type.h
+++ b/gcc/rust/resolve/rust-ast-resolve-type.h
@@ -312,9 +312,7 @@ public:
 
     // if it has a type lets resolve it
     if (param.has_type ())
-      {
-	ResolveType::go (param.get_type ().get (), param.get_node_id ());
-      }
+      ResolveType::go (param.get_type ().get (), param.get_node_id ());
 
     // for now lets focus on handling the basics: like struct<T> { a:T, ....}
     resolver->get_type_scope ().insert (
@@ -328,11 +326,7 @@ public:
   }
 
 private:
-  ResolveGenericParam (NodeId parent)
-    : ResolverBase (parent),
-
-      ok (false)
-  {}
+  ResolveGenericParam (NodeId parent) : ResolverBase (parent), ok (false) {}
 
   bool ok;
 };
diff --git a/gcc/rust/typecheck/rust-hir-const-fold-base.h b/gcc/rust/typecheck/rust-hir-const-fold-base.h
index 11eb5e4..5a59e70 100644
--- a/gcc/rust/typecheck/rust-hir-const-fold-base.h
+++ b/gcc/rust/typecheck/rust-hir-const-fold-base.h
@@ -120,7 +120,6 @@ public:
 
   virtual void visit (HIR::LifetimeWhereClauseItem &) override {}
   virtual void visit (HIR::TypeBoundWhereClauseItem &) override {}
-  virtual void visit (HIR::Method &) override {}
   virtual void visit (HIR::ModuleBodied &) override {}
   virtual void visit (HIR::ModuleNoBody &) override {}
   virtual void visit (HIR::ExternCrate &) override {}
diff --git a/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h b/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h
index e93e12a..47894b1 100644
--- a/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h
+++ b/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h
@@ -37,12 +37,6 @@ public:
     return resolver.ok;
   }
 
-  void visit (HIR::Method &method) override
-  {
-    ok = true;
-    result.assign (method.get_method_name ());
-  }
-
   void visit (HIR::Function &function) override
   {
     ok = true;
@@ -88,12 +82,6 @@ public:
     locus = function.get_locus ();
   }
 
-  void visit (HIR::Method &method) override
-  {
-    ok = true;
-    locus = method.get_locus ();
-  }
-
 private:
   GetLocusFromImplItem (Location &locus)
     : TypeCheckBase (), ok (false), locus (locus)
diff --git a/gcc/rust/typecheck/rust-hir-path-probe.h b/gcc/rust/typecheck/rust-hir-path-probe.h
index 06e8906..0e26778 100644
--- a/gcc/rust/typecheck/rust-hir-path-probe.h
+++ b/gcc/rust/typecheck/rust-hir-path-probe.h
@@ -95,21 +95,6 @@ public:
       }
   }
 
-  void visit (HIR::Method &method) override
-  {
-    Identifier name = method.get_method_name ();
-    if (search.as_string ().compare (name) == 0)
-      {
-	HirId tyid = method.get_mappings ().get_hirid ();
-	TyTy::BaseType *ty = nullptr;
-	bool ok = context->lookup_type (tyid, &ty);
-	rust_assert (ok);
-
-	PathProbeCandidate candidate{&method, ty};
-	candidates.push_back (std::move (candidate));
-      }
-  }
-
 private:
   PathProbeType (TyTy::BaseType *receiver, const HIR::PathIdentSegment &query)
     : TypeCheckBase (), receiver (receiver), search (query)
@@ -147,11 +132,6 @@ public:
     r.add_range (function.get_locus ());
   }
 
-  void visit (HIR::Method &method) override
-  {
-    r.add_range (method.get_locus ());
-  }
-
 private:
   ReportMultipleCandidateError (RichLocation &r) : TypeCheckBase (), r (r) {}
 
diff --git a/gcc/rust/typecheck/rust-hir-trait-resolve.h b/gcc/rust/typecheck/rust-hir-trait-resolve.h
index 2e358d3..6d926f7 100644
--- a/gcc/rust/typecheck/rust-hir-trait-resolve.h
+++ b/gcc/rust/typecheck/rust-hir-trait-resolve.h
@@ -33,9 +33,11 @@ class ResolveTraitItemToRef : public TypeCheckBase
   using Rust::Resolver::TypeCheckBase::visit;
 
 public:
-  static TraitItemReference Resolve (HIR::TraitItem &item)
+  static TraitItemReference
+  Resolve (HIR::TraitItem &item, TyTy::BaseType *self,
+	   std::vector<TyTy::SubstitutionParamMapping> substitutions)
   {
-    ResolveTraitItemToRef resolver;
+    ResolveTraitItemToRef resolver (self, substitutions);
     item.accept_vis (resolver);
     return resolver.resolved;
   }
@@ -89,7 +91,6 @@ public:
     if (!context->lookup_type (fn.get_mappings ().get_hirid (), &ty))
       {
 	HIR::TraitFunctionDecl &function = fn.get_decl ();
-	std::vector<TyTy::SubstitutionParamMapping> substitutions;
 	if (function.has_generics ())
 	  {
 	    for (auto &generic_param : function.get_generic_params ())
@@ -135,6 +136,21 @@ public:
 	  }
 
 	std::vector<std::pair<HIR::Pattern *, TyTy::BaseType *> > params;
+	if (function.is_method ())
+	  {
+	    // add the synthetic self param at the front, this is a placeholder
+	    // for compilation to know parameter names. The types are ignored
+	    // but we reuse the HIR identifier pattern which requires it
+	    HIR::SelfParam &self_param = function.get_self ();
+	    HIR::IdentifierPattern *self_pattern = new HIR::IdentifierPattern (
+	      "self", self_param.get_locus (), self_param.is_ref (),
+	      self_param.is_mut (), std::unique_ptr<HIR::Pattern> (nullptr));
+	    context->insert_type (self_param.get_mappings (), self->clone ());
+	    params.push_back (
+	      std::pair<HIR::Pattern *, TyTy::BaseType *> (self_pattern,
+							   self->clone ()));
+	  }
+
 	for (auto &param : function.get_function_params ())
 	  {
 	    // get the name as well required for later on
@@ -146,9 +162,9 @@ public:
 	  }
 
 	ty = new TyTy::FnType (fn.get_mappings ().get_hirid (),
-			       function.get_function_name (), false,
-			       std::move (params), ret_type,
-			       std::move (substitutions));
+			       function.get_function_name (),
+			       function.is_method (), std::move (params),
+			       ret_type, std::move (substitutions));
 	context->insert_type (fn.get_mappings (), ty);
       }
 
@@ -163,11 +179,16 @@ public:
   }
 
 private:
-  ResolveTraitItemToRef ()
-    : TypeCheckBase (), resolved (TraitItemReference::error ())
+  ResolveTraitItemToRef (
+    TyTy::BaseType *self,
+    std::vector<TyTy::SubstitutionParamMapping> substitutions)
+    : TypeCheckBase (), resolved (TraitItemReference::error ()), self (self),
+      substitutions (substitutions)
   {}
 
   TraitItemReference resolved;
+  TyTy::BaseType *self;
+  std::vector<TyTy::SubstitutionParamMapping> substitutions;
 };
 
 class TraitResolver : public TypeCheckBase
@@ -224,11 +245,41 @@ private:
 	return tref;
       }
 
+    TyTy::BaseType *self = nullptr;
+    std::vector<TyTy::SubstitutionParamMapping> substitutions;
+    for (auto &generic_param : trait_reference->get_generic_params ())
+      {
+	switch (generic_param.get ()->get_kind ())
+	  {
+	  case HIR::GenericParam::GenericKind::LIFETIME:
+	    // Skipping Lifetime completely until better handling.
+	    break;
+
+	    case HIR::GenericParam::GenericKind::TYPE: {
+	      auto param_type
+		= TypeResolveGenericParam::Resolve (generic_param.get ());
+	      context->insert_type (generic_param->get_mappings (), param_type);
+
+	      auto &typaram = static_cast<HIR::TypeParam &> (*generic_param);
+	      substitutions.push_back (
+		TyTy::SubstitutionParamMapping (typaram, param_type));
+
+	      if (typaram.get_type_representation ().compare ("Self") == 0)
+		{
+		  self = param_type;
+		}
+	    }
+	    break;
+	  }
+      }
+
+    rust_assert (self != nullptr);
+
     std::vector<TraitItemReference> item_refs;
     for (auto &item : trait_reference->get_trait_items ())
       {
 	TraitItemReference trait_item_ref
-	  = ResolveTraitItemToRef::Resolve (*item.get ());
+	  = ResolveTraitItemToRef::Resolve (*item.get (), self, substitutions);
 	item_refs.push_back (std::move (trait_item_ref));
       }
 
diff --git a/gcc/rust/typecheck/rust-hir-type-check-base.h b/gcc/rust/typecheck/rust-hir-type-check-base.h
index b2b11d3..ae71611 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-base.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-base.h
@@ -119,7 +119,6 @@ public:
 
   virtual void visit (HIR::LifetimeWhereClauseItem &) override {}
   virtual void visit (HIR::TypeBoundWhereClauseItem &) override {}
-  virtual void visit (HIR::Method &) override {}
   virtual void visit (HIR::ModuleBodied &) override {}
   virtual void visit (HIR::ModuleNoBody &) override {}
   virtual void visit (HIR::ExternCrate &) override {}
diff --git a/gcc/rust/typecheck/rust-hir-type-check-implitem.h b/gcc/rust/typecheck/rust-hir-type-check-implitem.h
index 75ba095..d161586 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-implitem.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-implitem.h
@@ -100,87 +100,22 @@ public:
       }
 
     std::vector<std::pair<HIR::Pattern *, TyTy::BaseType *> > params;
-    for (auto &param : function.get_function_params ())
+    if (function.is_method ())
       {
-	// get the name as well required for later on
-	auto param_tyty = TypeCheckType::Resolve (param.get_type ());
+	// add the synthetic self param at the front, this is a placeholder for
+	// compilation to know parameter names. The types are ignored but we
+	// reuse the HIR identifier pattern which requires it
+	HIR::SelfParam &self_param = function.get_self_param ();
+	HIR::IdentifierPattern *self_pattern = new HIR::IdentifierPattern (
+	  "self", self_param.get_locus (), self_param.is_ref (),
+	  self_param.is_mut (), std::unique_ptr<HIR::Pattern> (nullptr));
+	context->insert_type (self_param.get_mappings (), self->clone ());
 	params.push_back (
-	  std::pair<HIR::Pattern *, TyTy::BaseType *> (param.get_param_name (),
-						       param_tyty));
-
-	context->insert_type (param.get_mappings (), param_tyty);
-      }
-
-    auto fnType = new TyTy::FnType (function.get_mappings ().get_hirid (),
-				    function.get_function_name (), false,
-				    std::move (params), ret_type,
-				    std::move (substitutions));
-    context->insert_type (function.get_mappings (), fnType);
-  }
-
-  void visit (HIR::Method &method) override
-  {
-    if (method.has_generics ())
-      {
-	for (auto &generic_param : method.get_generic_params ())
-	  {
-	    switch (generic_param.get ()->get_kind ())
-	      {
-	      case HIR::GenericParam::GenericKind::LIFETIME:
-		// Skipping Lifetime completely until better handling.
-		break;
-
-		case HIR::GenericParam::GenericKind::TYPE: {
-		  auto param_type
-		    = TypeResolveGenericParam::Resolve (generic_param.get ());
-		  context->insert_type (generic_param->get_mappings (),
-					param_type);
-
-		  substitutions.push_back (TyTy::SubstitutionParamMapping (
-		    static_cast<HIR::TypeParam &> (*generic_param),
-		    param_type));
-		}
-		break;
-	      }
-	  }
+	  std::pair<HIR::Pattern *, TyTy::BaseType *> (self_pattern,
+						       self->clone ()));
       }
 
-    TyTy::BaseType *ret_type = nullptr;
-    if (!method.has_function_return_type ())
-      ret_type = new TyTy::TupleType (method.get_mappings ().get_hirid ());
-    else
-      {
-	auto resolved
-	  = TypeCheckType::Resolve (method.get_return_type ().get ());
-	if (resolved == nullptr)
-	  {
-	    rust_error_at (method.get_locus (),
-			   "failed to resolve return type");
-	    return;
-	  }
-
-	ret_type = resolved->clone ();
-	ret_type->set_ref (
-	  method.get_return_type ()->get_mappings ().get_hirid ());
-      }
-
-    // hold all the params to the fndef
-    std::vector<std::pair<HIR::Pattern *, TyTy::BaseType *> > params;
-
-    // add the synthetic self param at the front, this is a placeholder for
-    // compilation to know parameter names. The types are ignored but we reuse
-    // the HIR identifier pattern which requires it
-    HIR::SelfParam &self_param = method.get_self_param ();
-    HIR::IdentifierPattern *self_pattern
-      = new HIR::IdentifierPattern ("self", self_param.get_locus (),
-				    self_param.is_ref (), self_param.is_mut (),
-				    std::unique_ptr<HIR::Pattern> (nullptr));
-    context->insert_type (self_param.get_mappings (), self->clone ());
-    params.push_back (
-      std::pair<HIR::Pattern *, TyTy::BaseType *> (self_pattern,
-						   self->clone ()));
-
-    for (auto &param : method.get_function_params ())
+    for (auto &param : function.get_function_params ())
       {
 	// get the name as well required for later on
 	auto param_tyty = TypeCheckType::Resolve (param.get_type ());
@@ -191,11 +126,11 @@ public:
 	context->insert_type (param.get_mappings (), param_tyty);
       }
 
-    auto fnType
-      = new TyTy::FnType (method.get_mappings ().get_hirid (),
-			  method.get_method_name (), true, std::move (params),
-			  ret_type, std::move (substitutions));
-    context->insert_type (method.get_mappings (), fnType);
+    auto fnType = new TyTy::FnType (function.get_mappings ().get_hirid (),
+				    function.get_function_name (),
+				    function.is_method (), std::move (params),
+				    ret_type, std::move (substitutions));
+    context->insert_type (function.get_mappings (), fnType);
   }
 
 private:
@@ -249,36 +184,6 @@ public:
     expected_ret_tyty->unify (block_expr_ty);
   }
 
-  void visit (HIR::Method &method) override
-  {
-    TyTy::BaseType *lookup;
-    if (!context->lookup_type (method.get_mappings ().get_hirid (), &lookup))
-      {
-	rust_error_at (method.get_locus (), "failed to lookup function type");
-	return;
-      }
-
-    if (lookup->get_kind () != TyTy::TypeKind::FNDEF)
-      {
-	rust_error_at (method.get_locus (),
-		       "found invalid type for function [%s]",
-		       lookup->as_string ().c_str ());
-	return;
-      }
-
-    // need to get the return type from this
-    TyTy::FnType *resolve_fn_type = (TyTy::FnType *) lookup;
-    auto expected_ret_tyty = resolve_fn_type->get_return_type ();
-    context->push_return_type (expected_ret_tyty);
-
-    auto block_expr_ty
-      = TypeCheckExpr::Resolve (method.get_definition ().get (), false);
-
-    context->pop_return_type ();
-
-    expected_ret_tyty->unify (block_expr_ty);
-  }
-
 protected:
   TypeCheckImplItem (TyTy::BaseType *self) : TypeCheckBase (), self (self) {}
 
@@ -303,8 +208,6 @@ public:
 
   void visit (HIR::TypeAlias &type) override { gcc_unreachable (); }
 
-  void visit (HIR::Method &method) override { gcc_unreachable (); }
-
   void visit (HIR::Function &function) override
   {
     TypeCheckImplItem::visit (function);
@@ -333,8 +236,34 @@ public:
 	return;
       }
 
+    rust_assert (trait_item_ref.get_tyty ()->get_kind ()
+		 == TyTy::TypeKind::FNDEF);
+    TyTy::FnType *trait_item_fntype
+      = static_cast<TyTy::FnType *> (trait_item_ref.get_tyty ());
+
+    // sets substitute self into the trait_item_ref->tyty
+    TyTy::SubstitutionParamMapping *self_mapping = nullptr;
+    for (auto &param_mapping : trait_item_fntype->get_substs ())
+      {
+	const HIR::TypeParam &type_param = param_mapping.get_generic_param ();
+	if (type_param.get_type_representation ().compare ("Self") == 0)
+	  {
+	    self_mapping = &param_mapping;
+	    break;
+	  }
+      }
+    rust_assert (self_mapping != nullptr);
+
+    std::vector<TyTy::SubstitutionArg> mappings;
+    mappings.push_back (TyTy::SubstitutionArg (self_mapping, self));
+
+    TyTy::SubstitutionArgumentMappings implicit_self_substs (
+      mappings, function.get_locus ());
+    trait_item_fntype
+      = trait_item_fntype->handle_substitions (implicit_self_substs);
+
     // check the types are compatible
-    if (!trait_item_ref.get_tyty ()->can_eq (fntype))
+    if (!trait_item_fntype->can_eq (fntype))
       {
 	RichLocation r (function.get_locus ());
 	r.add_range (trait_item_ref.get_locus ());
diff --git a/gcc/rust/typecheck/rust-tycheck-dump.h b/gcc/rust/typecheck/rust-tycheck-dump.h
index 11f0c3a..b80372b 100644
--- a/gcc/rust/typecheck/rust-tycheck-dump.h
+++ b/gcc/rust/typecheck/rust-tycheck-dump.h
@@ -84,18 +84,6 @@ public:
     dump += indent () + "}\n";
   }
 
-  void visit (HIR::Method &method) override
-  {
-    dump += indent () + "fn " + method.get_method_name () + " "
-	    + type_string (method.get_mappings ()) + "\n";
-    dump += indent () + "{\n";
-
-    HIR::BlockExpr *function_body = method.get_function_body ().get ();
-    function_body->accept_vis (*this);
-
-    dump += indent () + "}\n";
-  }
-
   void visit (HIR::BlockExpr &expr) override
   {
     indentation_level++;
diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h
index 51ede17..438c52b 100644
--- a/gcc/rust/typecheck/rust-tyty.h
+++ b/gcc/rust/typecheck/rust-tyty.h
@@ -468,7 +468,6 @@ class SubstitutionParamMapping
 {
 public:
   SubstitutionParamMapping (const HIR::TypeParam &generic, ParamType *param)
-
     : generic (generic), param (param)
   {}
 
diff --git a/gcc/testsuite/rust/compile/torture/traits3.rs b/gcc/testsuite/rust/compile/torture/traits3.rs
new file mode 100644
index 0000000..621fcde
--- /dev/null
+++ b/gcc/testsuite/rust/compile/torture/traits3.rs
@@ -0,0 +1,18 @@
+pub trait Foo {
+    fn Bar(self) -> i32;
+    // { dg-warning "unused name .self." "" { target *-*-* } .-1 }
+    // { dg-warning "unused name .Bar." "" { target *-*-* } .-2 }
+}
+
+struct Baz;
+// { dg-warning "struct is never constructed: .Baz." "" { target *-*-* } .-1 }
+
+impl Foo for Baz {
+    fn Bar(self) -> i32 {
+        // { dg-warning "unused name .self." "" { target *-*-* } .-1 }
+        // { dg-warning "unused name .<Baz as Foo>::Bar." "" { target *-*-* } .-2 }
+        123
+    }
+}
+
+fn main() {}