Merge #1184

1184: Fix bad name resolution on path with generic segments r=philberty a=philberty

This fix breaks down into two commits the first commit fixes the issue directly
then the second one fixes the regression that was introduced in the first.

The TLDR for this patch set is that when we resolve the path here:

  module::transmute

The name-resolver wronly added the specified generic arguments into the canonical
path meaning that the root segment could not be name-resolved such that during
type resolution we iterate and apply the generic arguments appropriately to the root
segment.

In fixing this it introduced a regression exposing a few more complex issues with
inference variables but for a more complete explanation about that please read the
commit message from ea38a59

Fixes #1173 

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

Author: bors[bot]

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

@@ -262,6 +262,8 @@ class PathPattern : public Pattern
 
   std::vector<PathExprSegment> &get_segments () { return segments; }
 
+  const std::vector<PathExprSegment> &get_segments () const { return segments; }
+
   PathExprSegment &get_root_seg () { return segments.at (0); }
 
   PathExprSegment get_final_segment () const { return segments.back (); }

gcc/rust/resolve/rust-ast-resolve-type.h

@@ -169,13 +169,8 @@ class ResolvePathSegmentToCanonicalPath
 	return CanonicalPath::create_empty ();
       }
 
-    std::string generics
-      = ResolveTypeToCanonicalPath::canonicalize_generic_args (
-	seg.get_generic_args ());
-
     return CanonicalPath::new_seg (seg.get_node_id (),
-				   seg.get_ident_segment ().as_string ()
-				     + "::" + generics);
+				   seg.get_ident_segment ().as_string ());
   }
 };
 

gcc/rust/typecheck/rust-hir-type-check-expr.h

@@ -221,13 +221,6 @@ class TypeCheckExpr : public TypeCheckBase
 
     infered
       = TyTy::TypeCheckCallExpr::go (function_tyty, expr, variant, context);
-    if (infered == nullptr)
-      {
-	rust_error_at (expr.get_locus (), "failed to lookup type to CallExpr");
-	return;
-      }
-
-    infered->set_ref (expr.get_mappings ().get_hirid ());
   }
 
   void visit (HIR::MethodCallExpr &expr) override
@@ -1076,7 +1069,6 @@ class TypeCheckExpr : public TypeCheckBase
     : TypeCheckBase (), infered (nullptr), inside_loop (inside_loop)
   {}
 
-  // Beware: currently returns Tyty::ErrorType or nullptr in case of error.
   TyTy::BaseType *resolve_root_path (HIR::PathInExpression &expr,
 				     size_t *offset,
 				     NodeId *root_resolved_node_id);

gcc/rust/typecheck/rust-hir-type-check-path.cc

@@ -138,27 +138,17 @@ void
 TypeCheckExpr::visit (HIR::PathInExpression &expr)
 {
   NodeId resolved_node_id = UNKNOWN_NODEID;
-
   size_t offset = -1;
   TyTy::BaseType *tyseg = resolve_root_path (expr, &offset, &resolved_node_id);
-  rust_assert (tyseg != nullptr);
-
   if (tyseg->get_kind () == TyTy::TypeKind::ERROR)
     return;
 
-  if (expr.get_num_segments () == 1)
-    {
-      Location locus = expr.get_segments ().back ().get_locus ();
-
-      bool is_big_self
-	= expr.get_segments ().front ().get_segment ().as_string ().compare (
-	    "Self")
-	  == 0;
-      if (!is_big_self && tyseg->needs_generic_substitutions ())
-	{
-	  tyseg = SubstMapper::InferSubst (tyseg, locus);
-	}
+  if (tyseg->needs_generic_substitutions ())
+    tyseg = SubstMapper::InferSubst (tyseg, expr.get_locus ());
 
+  bool fully_resolved = offset == expr.get_segments ().size ();
+  if (fully_resolved)
+    {
       infered = tyseg;
       return;
     }
@@ -171,14 +161,11 @@ TyTy::BaseType *
 TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
 				  NodeId *root_resolved_node_id)
 {
-  TyTy::BaseType *root_tyty = nullptr;
   *offset = 0;
   for (size_t i = 0; i < expr.get_num_segments (); i++)
     {
       HIR::PathExprSegment &seg = expr.get_segments ().at (i);
-
       bool have_more_segments = (expr.get_num_segments () - 1 != i);
-      bool is_root = *offset == 0 || root_tyty == nullptr;
       NodeId ast_node_id = seg.get_mappings ().get_nodeid ();
 
       // then lookup the reference_node_id
@@ -205,40 +192,30 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
       // ref_node_id is the NodeId that the segments refers to.
       if (ref_node_id == UNKNOWN_NODEID)
 	{
-	  if (is_root)
-	    {
-	      rust_error_at (seg.get_locus (),
-			     "failed to type resolve root segment");
-	      return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
-	    }
-	  return root_tyty;
+	  rust_error_at (seg.get_locus (),
+			 "failed to type resolve root segment");
+	  return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
 	}
 
       // node back to HIR
       HirId ref;
       if (!mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (),
 					 ref_node_id, &ref))
 	{
-	  if (is_root)
-	    {
-	      rust_error_at (seg.get_locus (), "456 reverse lookup failure");
-	      rust_debug_loc (
-		seg.get_locus (),
-		"failure with [%s] mappings [%s] ref_node_id [%u]",
-		seg.as_string ().c_str (),
-		seg.get_mappings ().as_string ().c_str (), ref_node_id);
+	  rust_error_at (seg.get_locus (), "456 reverse lookup failure");
+	  rust_debug_loc (seg.get_locus (),
+			  "failure with [%s] mappings [%s] ref_node_id [%u]",
+			  seg.as_string ().c_str (),
+			  seg.get_mappings ().as_string ().c_str (),
+			  ref_node_id);
 
-	      return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
-	    }
-
-	  return root_tyty;
+	  return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
 	}
 
       auto seg_is_module
 	= (nullptr
 	   != mappings->lookup_module (expr.get_mappings ().get_crate_num (),
 				       ref));
-
       if (seg_is_module)
 	{
 	  // A::B::C::this_is_a_module::D::E::F
@@ -261,33 +238,8 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
       TyTy::BaseType *lookup = nullptr;
       if (!context->lookup_type (ref, &lookup))
 	{
-	  if (is_root)
-	    {
-	      rust_error_at (seg.get_locus (),
-			     "failed to resolve root segment");
-	      return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
-	    }
-	  return root_tyty;
-	}
-
-      // if we have a previous segment type
-      if (root_tyty != nullptr)
-	{
-	  // if this next segment needs substitution we must apply the
-	  // previous type arguments
-	  //
-	  // such as: GenericStruct::<_>::new(123, 456)
-	  if (lookup->needs_generic_substitutions ())
-	    {
-	      if (!root_tyty->needs_generic_substitutions ())
-		{
-		  auto used_args_in_prev_segment
-		    = GetUsedSubstArgs::From (root_tyty);
-		  lookup
-		    = SubstMapperInternal::Resolve (lookup,
-						    used_args_in_prev_segment);
-		}
-	    }
+	  rust_error_at (seg.get_locus (), "failed to resolve root segment");
+	  return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
 	}
 
       // turbo-fish segment path::<ty>
@@ -300,16 +252,16 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
 			     lookup->as_string ().c_str ());
 	      return new TyTy::ErrorType (lookup->get_ref ());
 	    }
-	  lookup = SubstMapper::Resolve (lookup, expr.get_locus (),
+	  lookup = SubstMapper::Resolve (lookup, seg.get_locus (),
 					 &seg.get_generic_args ());
 	}
 
       *root_resolved_node_id = ref_node_id;
       *offset = *offset + 1;
-      root_tyty = lookup;
+      return lookup;
     }
 
-  return root_tyty;
+  return new TyTy::ErrorType (expr.get_mappings ().get_hirid ());
 }
 
 void
@@ -424,19 +376,11 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 	  bool ok = context->lookup_type (impl_ty_id, &impl_block_ty);
 	  rust_assert (ok);
 
-	  if (prev_segment->needs_generic_substitutions ())
-	    {
-	      if (!impl_block_ty->needs_generic_substitutions ())
-		{
-		  prev_segment = impl_block_ty;
-		}
-	      else
-		{
-		  HIR::PathExprSegment &pseg = segments.at (i - 1);
-		  Location locus = pseg.get_locus ();
-		  prev_segment = SubstMapper::InferSubst (prev_segment, locus);
-		}
-	    }
+	  if (impl_block_ty->needs_generic_substitutions ())
+	    impl_block_ty
+	      = SubstMapper::InferSubst (impl_block_ty, seg.get_locus ());
+
+	  prev_segment = prev_segment->unify (impl_block_ty);
 	}
 
       if (tyseg->needs_generic_substitutions ())

gcc/rust/typecheck/rust-hir-type-check.cc

@@ -54,18 +54,11 @@ TypeResolution::Resolve (HIR::Crate &crate)
 
   // default inference variables if possible
   context->iterate ([&] (HirId id, TyTy::BaseType *ty) mutable -> bool {
-    if (ty->get_kind () == TyTy::TypeKind::ERROR)
-      {
-	rust_error_at (mappings->lookup_location (id),
-		       "failure in type resolution for %u", id);
-	return false;
-      }
-
     // nothing to do
     if (ty->get_kind () != TyTy::TypeKind::INFER)
       return true;
 
-    TyTy::InferType *infer_var = (TyTy::InferType *) ty;
+    TyTy::InferType *infer_var = static_cast<TyTy::InferType *> (ty);
     TyTy::BaseType *default_type;
     bool ok = infer_var->default_type (&default_type);
     if (!ok)

gcc/rust/typecheck/rust-tyty-rules.h

@@ -89,6 +89,11 @@ class BaseRules : public TyVisitor
     for (auto ref : other->get_combined_refs ())
       resolved->append_reference (ref);
 
+    other->append_reference (resolved->get_ref ());
+    other->append_reference (get_base ()->get_ref ());
+    get_base ()->append_reference (resolved->get_ref ());
+    get_base ()->append_reference (other->get_ref ());
+
     bool result_resolved = resolved->get_kind () != TyTy::TypeKind::INFER;
     bool result_is_infer_var = resolved->get_kind () == TyTy::TypeKind::INFER;
     bool results_is_non_general_infer_var

gcc/rust/typecheck/rust-tyty.cc

@@ -263,6 +263,7 @@ TyVar::get_implicit_infer_var (Location locus)
 			infer);
   mappings->insert_location (mappings->get_current_crate (), infer->get_ref (),
 			     locus);
+
   return TyVar (infer->get_ref ());
 }
 
@@ -341,8 +342,34 @@ InferType::cast (BaseType *other)
 BaseType *
 InferType::clone () const
 {
-  return new InferType (get_ref (), get_ty_ref (), get_infer_kind (),
-			get_ident ().locus, get_combined_refs ());
+  // clones for inference variables are special in that they _must_ exist within
+  // the type check context and we must ensure we don't loose the chain
+  // otherwise we will end up in the missing type annotations case
+  //
+  // This means we cannot simply take over the same reference we must generate a
+  // new ref just like the get_implicit_infer_var code then we can setup the
+  // chain of references accordingly to ensure we don't loose the ability to
+  // update the inference variables when we solve the type
+
+  auto mappings = Analysis::Mappings::get ();
+  auto context = Resolver::TypeCheckContext::get ();
+
+  InferType *clone
+    = new InferType (mappings->get_next_hir_id (), get_infer_kind (),
+		     get_ident ().locus, get_combined_refs ());
+
+  context->insert_type (Analysis::NodeMapping (mappings->get_current_crate (),
+					       UNKNOWN_NODEID,
+					       clone->get_ref (),
+					       UNKNOWN_LOCAL_DEFID),
+			clone);
+  mappings->insert_location (mappings->get_current_crate (), clone->get_ref (),
+			     mappings->lookup_location (get_ref ()));
+
+  // setup the chain to reference this
+  clone->append_reference (get_ref ());
+
+  return clone;
 }
 
 bool

gcc/rust/typecheck/rust-tyty.h

@@ -955,12 +955,23 @@ class SubstitutionRef
   BaseType *infer_substitions (Location locus)
   {
     std::vector<SubstitutionArg> args;
+    std::map<std::string, BaseType *> argument_mappings;
     for (auto &p : get_substs ())
       {
 	if (p.needs_substitution ())
 	  {
-	    TyVar infer_var = TyVar::get_implicit_infer_var (locus);
-	    args.push_back (SubstitutionArg (&p, infer_var.get_tyty ()));
+	    const std::string &symbol = p.get_param_ty ()->get_symbol ();
+	    auto it = argument_mappings.find (symbol);
+	    if (it == argument_mappings.end ())
+	      {
+		TyVar infer_var = TyVar::get_implicit_infer_var (locus);
+		args.push_back (SubstitutionArg (&p, infer_var.get_tyty ()));
+		argument_mappings[symbol] = infer_var.get_tyty ();
+	      }
+	    else
+	      {
+		args.push_back (SubstitutionArg (&p, it->second));
+	      }
 	  }
 	else
 	  {

gcc/testsuite/rust/compile/issue-1173.rs

@@ -0,0 +1,20 @@
+// { dg-additional-options "-w" }
+mod mem {
+    extern "rust-intrinsic" {
+        fn transmute<U, V>(_: U) -> V;
+    }
+}
+
+pub trait Hasher {
+    fn write(&mut self, bytes: &[u8]);
+    fn write_u16(&mut self, i: u16) {
+        self.write(&mem::transmute::<_, [u8; 2]>(i))
+    }
+}
+
+pub struct SipHasher;
+
+impl Hasher for SipHasher {
+    #[inline]
+    fn write(&mut self, msg: &[u8]) {}
+}