[committed,010/103] gccrs: Ensure uniqueness on Path probe's

Message ID 20230221120230.596966-11-arthur.cohen@embecosm.com
State Unresolved
Headers
Series [committed,001/103] gccrs: Fix missing dead code analysis ICE on local enum definition |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Arthur Cohen Feb. 21, 2023, 12:01 p.m. UTC
  From: Philip Herron <philip.herron@embecosm.com>

When we lookup names in paths such as Foo::bar, foo is a type we resolve
and then we lookup 'bar' based on what type Foo is which includes probing
relevant bounds of this type. We currently return a vector of possible
candidates and this patch changes it so that we return a set of unique
items based on DefId.

Addresses #1555

gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc
	(CompileExpr::resolve_method_address): Use auto and minor change
	in candidate init.
	* typecheck/rust-hir-type-check-path.cc
	(TypeCheckExpr::resolve_segments): Likewise.
	* typecheck/rust-hir-type-check-type.cc: Likewise.
	* backend/rust-compile-resolve-path.cc
	(HIRCompileBase::query_compile): Likewise. Removecall to
	set_ty_ref.
	* typecheck/rust-hir-path-probe.h (struct PathProbeCandidate): Add
	locus initializer in ctor, implement get_defid.
	(class PathProbeType::Probe): return a set instead of vector.
	Adjust class impl.
	(class ReportMultipleCandidateError): Do not inherit from
	HIRImplVisitor anymore and remove corresponding impl. Adjust for
	change in Probe. Simplify Report handling.
	(class PathProbeImplTrait::Probe): Adjust return type.
---
 gcc/rust/backend/rust-compile-expr.cc         |   4 +-
 gcc/rust/backend/rust-compile-resolve-path.cc |   8 +-
 gcc/rust/typecheck/rust-hir-path-probe.h      | 108 ++++++++----------
 .../typecheck/rust-hir-type-check-path.cc     |   2 +-
 .../typecheck/rust-hir-type-check-type.cc     |   2 +-
 5 files changed, 58 insertions(+), 66 deletions(-)
  

Patch

diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index d58e2258947..ddf914f6736 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -1982,7 +1982,7 @@  CompileExpr::resolve_method_address (TyTy::FnType *fntype, HirId ref,
   // trait-impl-item's definition
 
   auto root = receiver->get_root ();
-  std::vector<Resolver::PathProbeCandidate> candidates
+  auto candidates
     = Resolver::PathProbeType::Probe (root, segment, true /* probe_impls */,
 				      false /* probe_bounds */,
 				      true /* ignore_mandatory_trait_items */);
@@ -2011,7 +2011,7 @@  CompileExpr::resolve_method_address (TyTy::FnType *fntype, HirId ref,
       // implementation and we should just return error_mark_node
 
       rust_assert (candidates.size () == 1);
-      auto &candidate = candidates.at (0);
+      auto &candidate = *candidates.begin ();
       rust_assert (candidate.is_impl_candidate ());
       rust_assert (candidate.ty->get_kind () == TyTy::TypeKind::FNDEF);
       TyTy::FnType *candidate_call = static_cast<TyTy::FnType *> (candidate.ty);
diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc
index 2cc9505692a..ab8e628c75c 100644
--- a/gcc/rust/backend/rust-compile-resolve-path.cc
+++ b/gcc/rust/backend/rust-compile-resolve-path.cc
@@ -251,7 +251,7 @@  HIRCompileBase::query_compile (HirId ref, TyTy::BaseType *lookup,
 	  // item so its up to us to figure out if this path should resolve
 	  // to an trait-impl-block-item or if it can be defaulted to the
 	  // trait-impl-item's definition
-	  std::vector<Resolver::PathProbeCandidate> candidates
+	  auto candidates
 	    = Resolver::PathProbeImplTrait::Probe (receiver, final_segment,
 						   trait_ref);
 	  if (candidates.size () == 0)
@@ -270,7 +270,9 @@  HIRCompileBase::query_compile (HirId ref, TyTy::BaseType *lookup,
 	    }
 	  else
 	    {
-	      Resolver::PathProbeCandidate &candidate = candidates.at (0);
+	      rust_assert (candidates.size () == 1);
+
+	      auto candidate = *candidates.begin ();
 	      rust_assert (candidate.is_impl_candidate ());
 
 	      HIR::ImplBlock *impl = candidate.item.impl.parent;
@@ -288,8 +290,6 @@  HIRCompileBase::query_compile (HirId ref, TyTy::BaseType *lookup,
 	      else
 		return CompileInherentImplItem::Compile (impl_item, ctx, lookup,
 							 true, expr_locus);
-
-	      lookup->set_ty_ref (impl_item->get_impl_mappings ().get_hirid ());
 	    }
 	}
     }
diff --git a/gcc/rust/typecheck/rust-hir-path-probe.h b/gcc/rust/typecheck/rust-hir-path-probe.h
index c957487dd2a..6d6bcf8e7cd 100644
--- a/gcc/rust/typecheck/rust-hir-path-probe.h
+++ b/gcc/rust/typecheck/rust-hir-path-probe.h
@@ -80,17 +80,17 @@  struct PathProbeCandidate
 
   PathProbeCandidate (CandidateType type, TyTy::BaseType *ty, Location locus,
 		      EnumItemCandidate enum_field)
-    : type (type), ty (ty), item (enum_field)
+    : type (type), ty (ty), locus (locus), item (enum_field)
   {}
 
   PathProbeCandidate (CandidateType type, TyTy::BaseType *ty, Location locus,
 		      ImplItemCandidate impl)
-    : type (type), ty (ty), item (impl)
+    : type (type), ty (ty), locus (locus), item (impl)
   {}
 
   PathProbeCandidate (CandidateType type, TyTy::BaseType *ty, Location locus,
 		      TraitItemCandidate trait)
-    : type (type), ty (ty), item (trait)
+    : type (type), ty (ty), locus (locus), item (trait)
   {}
 
   std::string as_string () const
@@ -123,12 +123,45 @@  struct PathProbeCandidate
   }
 
   bool is_error () const { return type == ERROR; }
+
+  DefId get_defid () const
+  {
+    switch (type)
+      {
+      case ENUM_VARIANT:
+	return item.enum_field.variant->get_defid ();
+	break;
+
+      case IMPL_CONST:
+      case IMPL_TYPE_ALIAS:
+      case IMPL_FUNC:
+	return item.impl.impl_item->get_impl_mappings ().get_defid ();
+	break;
+
+      case TRAIT_ITEM_CONST:
+      case TRAIT_TYPE_ALIAS:
+      case TRAIT_FUNC:
+	return item.trait.item_ref->get_mappings ().get_defid ();
+	break;
+
+      case ERROR:
+      default:
+	return UNKNOWN_DEFID;
+      }
+
+    return UNKNOWN_DEFID;
+  }
+
+  bool operator< (const PathProbeCandidate &c) const
+  {
+    return get_defid () < c.get_defid ();
+  }
 };
 
 class PathProbeType : public TypeCheckBase, public HIR::HIRImplVisitor
 {
 public:
-  static std::vector<PathProbeCandidate>
+  static std::set<PathProbeCandidate>
   Probe (const TyTy::BaseType *receiver,
 	 const HIR::PathIdentSegment &segment_name, bool probe_impls,
 	 bool probe_bounds, bool ignore_mandatory_trait_items,
@@ -203,7 +236,7 @@  public:
 	PathProbeCandidate candidate{
 	  PathProbeCandidate::CandidateType::IMPL_TYPE_ALIAS, ty,
 	  alias.get_locus (), impl_item_candidate};
-	candidates.push_back (std::move (candidate));
+	candidates.insert (std::move (candidate));
       }
   }
 
@@ -222,7 +255,7 @@  public:
 	PathProbeCandidate candidate{
 	  PathProbeCandidate::CandidateType::IMPL_CONST, ty,
 	  constant.get_locus (), impl_item_candidate};
-	candidates.push_back (std::move (candidate));
+	candidates.insert (std::move (candidate));
       }
   }
 
@@ -241,7 +274,7 @@  public:
 	PathProbeCandidate candidate{
 	  PathProbeCandidate::CandidateType::IMPL_FUNC, ty,
 	  function.get_locus (), impl_item_candidate};
-	candidates.push_back (std::move (candidate));
+	candidates.insert (std::move (candidate));
       }
   }
 
@@ -259,7 +292,7 @@  protected:
     PathProbeCandidate candidate{
       PathProbeCandidate::CandidateType::ENUM_VARIANT, receiver->clone (),
       mappings->lookup_location (adt->get_ty_ref ()), enum_item_candidate};
-    candidates.push_back (std::move (candidate));
+    candidates.insert (std::move (candidate));
   }
 
   void process_impl_items_for_candidates ()
@@ -338,8 +371,9 @@  protected:
 								impl};
 
     PathProbeCandidate candidate{candidate_type, trait_item_tyty,
-				 trait_ref->get_locus (), trait_item_candidate};
-    candidates.push_back (std::move (candidate));
+				 trait_item_ref->get_locus (),
+				 trait_item_candidate};
+    candidates.insert (std::move (candidate));
   }
 
   void
@@ -383,7 +417,7 @@  protected:
     PathProbeCandidate candidate{candidate_type, trait_item_tyty,
 				 trait_item_ref->get_locus (),
 				 trait_item_candidate};
-    candidates.push_back (std::move (candidate));
+    candidates.insert (std::move (candidate));
   }
 
 protected:
@@ -428,72 +462,30 @@  protected:
 
   const TyTy::BaseType *receiver;
   const HIR::PathIdentSegment &search;
-  std::vector<PathProbeCandidate> candidates;
+  std::set<PathProbeCandidate> candidates;
   HIR::ImplBlock *current_impl;
   DefId specific_trait_id;
 };
 
-class ReportMultipleCandidateError : private TypeCheckBase,
-				     private HIR::HIRImplVisitor
+class ReportMultipleCandidateError : private TypeCheckBase
 {
 public:
-  static void Report (std::vector<PathProbeCandidate> &candidates,
+  static void Report (std::set<PathProbeCandidate> &candidates,
 		      const HIR::PathIdentSegment &query, Location query_locus)
   {
     RichLocation r (query_locus);
-    ReportMultipleCandidateError visitor (r);
     for (auto &c : candidates)
-      {
-	switch (c.type)
-	  {
-	  case PathProbeCandidate::CandidateType::ERROR:
-	  case PathProbeCandidate::CandidateType::ENUM_VARIANT:
-	    gcc_unreachable ();
-	    break;
-
-	  case PathProbeCandidate::CandidateType::IMPL_CONST:
-	  case PathProbeCandidate::CandidateType::IMPL_TYPE_ALIAS:
-	  case PathProbeCandidate::CandidateType::IMPL_FUNC:
-	    c.item.impl.impl_item->accept_vis (visitor);
-	    break;
-
-	  case PathProbeCandidate::CandidateType::TRAIT_ITEM_CONST:
-	  case PathProbeCandidate::CandidateType::TRAIT_TYPE_ALIAS:
-	  case PathProbeCandidate::CandidateType::TRAIT_FUNC:
-	    r.add_range (c.item.trait.item_ref->get_locus ());
-	    break;
-	  }
-      }
+      r.add_range (c.locus);
 
     rust_error_at (r, "multiple applicable items in scope for: %s",
 		   query.as_string ().c_str ());
   }
-
-  void visit (HIR::TypeAlias &alias) override
-  {
-    r.add_range (alias.get_locus ());
-  }
-
-  void visit (HIR::ConstantItem &constant) override
-  {
-    r.add_range (constant.get_locus ());
-  }
-
-  void visit (HIR::Function &function) override
-  {
-    r.add_range (function.get_locus ());
-  }
-
-private:
-  ReportMultipleCandidateError (RichLocation &r) : TypeCheckBase (), r (r) {}
-
-  RichLocation &r;
 };
 
 class PathProbeImplTrait : public PathProbeType
 {
 public:
-  static std::vector<PathProbeCandidate>
+  static std::set<PathProbeCandidate>
   Probe (const TyTy::BaseType *receiver,
 	 const HIR::PathIdentSegment &segment_name,
 	 const TraitReference *trait_reference)
diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc
index 305d73f76f9..6f1fd416c19 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-path.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc
@@ -337,7 +337,7 @@  TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 	  return;
 	}
 
-      auto &candidate = candidates.at (0);
+      auto &candidate = *candidates.begin ();
       prev_segment = tyseg;
       tyseg = candidate.ty;
 
diff --git a/gcc/rust/typecheck/rust-hir-type-check-type.cc b/gcc/rust/typecheck/rust-hir-type-check-type.cc
index 5a6d00a0e9d..a91d15c8e2c 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-type.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-type.cc
@@ -462,7 +462,7 @@  TypeCheckType::resolve_segments (
 	  return new TyTy::ErrorType (expr_id);
 	}
 
-      auto &candidate = candidates.at (0);
+      auto &candidate = *candidates.begin ();
       prev_segment = tyseg;
       tyseg = candidate.ty;