Merge #1594 #1595

1594: Refactor TraitResolver to not require a visitor r=philberty a=philberty

We used a visitor to dispatch the HIR::Item so that we could cast it directly to an HIR::Trait and then check for nullptr if it failed. This patch changes this to simply use our new enum item_type_kind to switch so we can directly static_cast.


1595: Support outer attribute handling on trait items just like normal items r=philberty a=philberty

This patch adds a proxy class ItemWrapper to be a proxy allowing us to use the same code paths for attribute handling as we have with normal items. We need this so we can grab the fn trait associated type lang item's. Which are being missed currently.

Addresses #195



Co-authored-by: Philip Herron <[email protected]>

Author: bors[bot]

gcc/rust/hir/rust-ast-lower-base.cc

@@ -824,7 +824,7 @@ ASTLoweringBase::lower_qualifiers (const AST::FunctionQualifiers &qualifiers)
 }
 
 void
-ASTLoweringBase::handle_outer_attributes (const HIR::Item &item)
+ASTLoweringBase::handle_outer_attributes (const ItemWrapper &item)
 {
   for (const auto &attr : item.get_outer_attrs ())
     {
@@ -855,7 +855,7 @@ ASTLoweringBase::handle_outer_attributes (const HIR::Item &item)
 }
 
 void
-ASTLoweringBase::handle_doc_item_attribute (const HIR::Item &item,
+ASTLoweringBase::handle_doc_item_attribute (const ItemWrapper &item,
 					    const AST::Attribute &attr)
 {
   auto simple_doc_comment = attr.has_attr_input ()
@@ -878,7 +878,7 @@ ASTLoweringBase::handle_doc_item_attribute (const HIR::Item &item,
 }
 
 void
-ASTLoweringBase::handle_lang_item_attribute (const HIR::Item &item,
+ASTLoweringBase::handle_lang_item_attribute (const ItemWrapper &item,
 					     const AST::Attribute &attr)
 {
   auto &literal = static_cast<AST::AttrInputLiteral &> (attr.get_attr_input ());

gcc/rust/hir/rust-ast-lower-base.h

@@ -29,6 +29,30 @@
 namespace Rust {
 namespace HIR {
 
+// proxy class so we can do attribute checking on items and trait items
+class ItemWrapper
+{
+public:
+  ItemWrapper (const HIR::Item &item)
+    : mappings (item.get_mappings ()), locus (item.get_locus ()),
+      outer_attrs (item.get_outer_attrs ())
+  {}
+
+  ItemWrapper (const HIR::TraitItem &item)
+    : mappings (item.get_mappings ()), locus (item.get_trait_locus ()),
+      outer_attrs (item.get_outer_attrs ())
+  {}
+
+  const Analysis::NodeMapping &get_mappings () const { return mappings; }
+  Location get_locus () const { return locus; }
+  const AST::AttrVec &get_outer_attrs () const { return outer_attrs; }
+
+private:
+  const Analysis::NodeMapping &mappings;
+  Location locus;
+  const AST::AttrVec &outer_attrs;
+};
+
 // base class to allow derivatives to overload as needed
 class ASTLoweringBase : public AST::ASTVisitor
 {
@@ -264,12 +288,12 @@ class ASTLoweringBase : public AST::ASTVisitor
   HIR::FunctionQualifiers
   lower_qualifiers (const AST::FunctionQualifiers &qualifiers);
 
-  void handle_outer_attributes (const HIR::Item &item);
+  void handle_outer_attributes (const ItemWrapper &item);
 
-  void handle_lang_item_attribute (const HIR::Item &item,
+  void handle_lang_item_attribute (const ItemWrapper &item,
 				   const AST::Attribute &attr);
 
-  void handle_doc_item_attribute (const HIR::Item &item,
+  void handle_doc_item_attribute (const ItemWrapper &item,
 				  const AST::Attribute &attr);
 
   bool is_known_attribute (const std::string &attribute_path) const;

gcc/rust/hir/rust-ast-lower-implitem.h

@@ -305,16 +305,14 @@ class ASTLowerTraitItem : public ASTLoweringBase
 
     if (resolver.translated != nullptr)
       {
-	// FIXME
+	auto id = resolver.translated->get_mappings ().get_hirid ();
+	auto defid = resolver.translated->get_mappings ().get_defid ();
+	auto locus = resolver.translated->get_trait_locus ();
 
-	// auto id = resolver.translated->get_mappings ().get_hirid ();
-	// auto defid = resolver.translated->get_mappings ().get_defid ();
-	// auto locus = resolver.translated->get_locus ();
-
-	// resolver.handle_outer_attributes (*resolver.translated);
+	resolver.handle_outer_attributes (*resolver.translated);
 	resolver.mappings->insert_hir_trait_item (resolver.translated);
-	// resolver.mappings->insert_location (id, locus);
-	// resolver.mappings->insert_defid_mapping (defid, resolver.item_cast);
+	resolver.mappings->insert_location (id, locus);
+	resolver.mappings->insert_defid_mapping (defid, resolver.translated);
       }
 
     return resolver.translated;

gcc/rust/hir/tree/rust-hir-item.h

@@ -2394,6 +2394,8 @@ class TraitItemFunc : public TraitItem
     return outer_attrs;
   }
 
+  Location get_trait_locus () const override { return get_locus (); }
+
 protected:
   // Clone function implementation as (not pure) virtual method
   TraitItemFunc *clone_trait_item_impl () const override
@@ -2480,6 +2482,8 @@ class TraitItemConst : public TraitItem
     return outer_attrs;
   }
 
+  Location get_trait_locus () const override { return get_locus (); }
+
 protected:
   // Clone function implementation as (not pure) virtual method
   TraitItemConst *clone_trait_item_impl () const override
@@ -2567,6 +2571,8 @@ class TraitItemType : public TraitItem
     return outer_attrs;
   }
 
+  Location get_trait_locus () const override { return get_locus (); }
+
 protected:
   // Clone function implementation as (not pure) virtual method
   TraitItemType *clone_trait_item_impl () const override

gcc/rust/hir/tree/rust-hir.h

@@ -796,7 +796,9 @@ class TraitItem : public Node
 
   virtual const std::string trait_identifier () const = 0;
 
-  const Analysis::NodeMapping get_mappings () const { return mappings; }
+  const Analysis::NodeMapping &get_mappings () const { return mappings; }
+
+  virtual Location get_trait_locus () const = 0;
 
   virtual TraitItemKind get_item_kind () const = 0;
 

gcc/rust/typecheck/rust-hir-trait-resolve.cc

@@ -91,33 +91,42 @@ TraitResolver::Lookup (HIR::TypePath &path)
   return resolver.lookup_path (path);
 }
 
-TraitResolver::TraitResolver ()
-  : TypeCheckBase (), resolved_trait_reference (nullptr)
-{}
+TraitResolver::TraitResolver () : TypeCheckBase () {}
 
-TraitReference *
-TraitResolver::resolve_path (HIR::TypePath &path)
+bool
+TraitResolver::resolve_path_to_trait (const HIR::TypePath &path,
+				      HIR::Trait **resolved) const
 {
   NodeId ref;
   if (!resolver->lookup_resolved_type (path.get_mappings ().get_nodeid (),
 				       &ref))
     {
       rust_error_at (path.get_locus (), "Failed to resolve path to node-id");
-      return &TraitReference::error_node ();
+      return false;
     }
 
   HirId hir_node = UNKNOWN_HIRID;
   if (!mappings->lookup_node_to_hir (ref, &hir_node))
     {
       rust_error_at (path.get_locus (), "Failed to resolve path to hir-id");
-      return &TraitReference::error_node ();
+      return false;
     }
 
   HIR::Item *resolved_item = mappings->lookup_hir_item (hir_node);
-
   rust_assert (resolved_item != nullptr);
-  resolved_item->accept_vis (*this);
-  rust_assert (resolved_trait_reference != nullptr);
+  rust_assert (resolved_item->get_item_kind () == HIR::Item::ItemKind::Trait);
+  *resolved = static_cast<HIR::Trait *> (resolved_item);
+
+  return true;
+}
+
+TraitReference *
+TraitResolver::resolve_path (HIR::TypePath &path)
+{
+  HIR::Trait *resolved_trait_reference;
+  bool ok = resolve_path_to_trait (path, &resolved_trait_reference);
+  if (!ok)
+    return &TraitReference::error_node ();
 
   return resolve_trait (resolved_trait_reference);
 }
@@ -237,26 +246,10 @@ TraitResolver::resolve_trait (HIR::Trait *trait_reference)
 TraitReference *
 TraitResolver::lookup_path (HIR::TypePath &path)
 {
-  NodeId ref;
-  if (!resolver->lookup_resolved_type (path.get_mappings ().get_nodeid (),
-				       &ref))
-    {
-      rust_error_at (path.get_locus (), "Failed to resolve path to node-id");
-      return &TraitReference::error_node ();
-    }
-
-  HirId hir_node = UNKNOWN_HIRID;
-  if (!mappings->lookup_node_to_hir (ref, &hir_node))
-    {
-      rust_error_at (path.get_locus (), "Failed to resolve path to hir-id");
-      return &TraitReference::error_node ();
-    }
-
-  HIR::Item *resolved_item = mappings->lookup_hir_item (hir_node);
-
-  rust_assert (resolved_item != nullptr);
-  resolved_item->accept_vis (*this);
-  rust_assert (resolved_trait_reference != nullptr);
+  HIR::Trait *resolved_trait_reference;
+  bool ok = resolve_path_to_trait (path, &resolved_trait_reference);
+  if (!ok)
+    return &TraitReference::error_node ();
 
   TraitReference *tref = &TraitReference::error_node ();
   if (context->lookup_trait_reference (

gcc/rust/typecheck/rust-hir-trait-resolve.h

@@ -55,10 +55,8 @@ class ResolveTraitItemToRef : public TypeCheckBase,
   std::vector<TyTy::SubstitutionParamMapping> substitutions;
 };
 
-class TraitResolver : public TypeCheckBase, private HIR::HIRFullVisitorBase
+class TraitResolver : public TypeCheckBase
 {
-  using HIR::HIRFullVisitorBase::visit;
-
 public:
   static TraitReference *Resolve (HIR::TypePath &path);
 
@@ -75,10 +73,8 @@ class TraitResolver : public TypeCheckBase, private HIR::HIRFullVisitorBase
 
   TraitReference *lookup_path (HIR::TypePath &path);
 
-  HIR::Trait *resolved_trait_reference;
-
-public:
-  void visit (HIR::Trait &trait) override { resolved_trait_reference = &trait; }
+  bool resolve_path_to_trait (const HIR::TypePath &path,
+			      HIR::Trait **resolved) const;
 };
 
 } // namespace Resolver

gcc/rust/util/rust-hir-map.cc

@@ -316,6 +316,7 @@ Mappings::insert_defid_mapping (DefId id, HIR::Item *item)
 
   rust_assert (lookup_defid (id) == nullptr);
   rust_assert (lookup_local_defid (crate_num, local_def_id) == nullptr);
+  rust_assert (lookup_trait_item_defid (id) == nullptr);
 
   defIdMappings[id] = item;
   insert_local_defid_mapping (crate_num, local_def_id, item);
@@ -331,6 +332,29 @@ Mappings::lookup_defid (DefId id)
   return it->second;
 }
 
+void
+Mappings::insert_defid_mapping (DefId id, HIR::TraitItem *item)
+{
+  CrateNum crate_num = id.crateNum;
+  LocalDefId local_def_id = id.localDefId;
+
+  rust_assert (lookup_defid (id) == nullptr);
+  rust_assert (lookup_local_defid (crate_num, local_def_id) == nullptr);
+  rust_assert (lookup_trait_item_defid (id) == nullptr);
+
+  defIdTraitItemMappings[id] = item;
+}
+
+HIR::TraitItem *
+Mappings::lookup_trait_item_defid (DefId id)
+{
+  auto it = defIdTraitItemMappings.find (id);
+  if (it == defIdTraitItemMappings.end ())
+    return nullptr;
+
+  return it->second;
+}
+
 void
 Mappings::insert_hir_item (HIR::Item *item)
 {

gcc/rust/util/rust-hir-map.h

@@ -104,6 +104,8 @@ class Mappings
 
   void insert_defid_mapping (DefId id, HIR::Item *item);
   HIR::Item *lookup_defid (DefId id);
+  void insert_defid_mapping (DefId id, HIR::TraitItem *item);
+  HIR::TraitItem *lookup_trait_item_defid (DefId id);
 
   void insert_local_defid_mapping (CrateNum crateNum, LocalDefId id,
 				   HIR::Item *item);
@@ -307,6 +309,7 @@ class Mappings
   std::map<CrateNum, AST::Crate *> ast_crate_mappings;
   std::map<CrateNum, HIR::Crate *> hir_crate_mappings;
   std::map<DefId, HIR::Item *> defIdMappings;
+  std::map<DefId, HIR::TraitItem *> defIdTraitItemMappings;
   std::map<CrateNum, std::map<LocalDefId, HIR::Item *>> localDefIdMappings;
 
   std::map<HirId, HIR::Module *> hirModuleMappings;