Fix regression in fix for #1173

A regression was introduced in e476433c45624ec713395c85bcbd410806e90639
this was a tricky bug to fix. There are several fixes here but they all
rely on one another so in order to have a commit which does not introduce
more regressions it seems best to keep these fixes together.

In this patch we remove the check at the end of type resolution to look
for any error type nodes within the type context since this will only ever
produce false postive/duplicate errors. Since any real error will have been
emitted at the time of occurance.

We also fixed a bug in infering the types on a generic type so for example:

  struct Foo<A,B>(A,B);
  impl<T> Foo<T,T> { ... }

In this example the Self type on the impl block will be Foo<T,T> so in the
code for infering the arguments for the type we simply iterated each of the
generic parameter mappings and if any of them need substitution we generate
implict inference variables for them but in this case the ParamType is T
so generating one for each of these T's will introduce a stay inference
variable which we cannot infer so it will not be used. This patch keeps
a mapping inside SubstitutionRef::infer_substitions to ensure we don't
introduce any extra inference variables than required.

The final fix was around how we clone inference variables, there is a
comment explaining this but we cannot safely clone a inference variables
simply by creating a new object, inference variables are closely tied to
the type-conext so that when we infer the type to be a concrete type we
need to be able to update the reference in memory but simply cloning an
inference variable does not guarentee this to occur. This change ensures
that we create a new implict inference variable and setup the reference
and chain apropirately.

Author: philberty

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/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/torture/issue-893-2.rs

@@ -24,7 +24,7 @@ impl Baz<i32, f32> {
 
 pub fn main() {
     let a = Foo::<i32>::new::<f32>(123, 456f32);
-    // let b = Foo::new::<f32>(123, 456f32);
+    let b = Foo::new::<f32>(123, 456f32);
 
     let c = Bar::<i32>(123);
     let d = Bar::baz(c);