[gcc/devel/rust/master] Handle generic substitution on path expressions

[gcc/devel/rust/master] Handle generic substitution on path expressions

[Thomas Schwinge]

https://gcc.gnu.org/g:257bf55827474e500d83d96b7a8f8ebacbd11e7d

commit 257bf55827474e500d83d96b7a8f8ebacbd11e7d
Author: Philip Herron <philip.herron@embecosm.com>
Date:   Tue Feb 8 11:12:32 2022 +0000

    Handle generic substitution on path expressions
    
    In this bug the path expression failed to take Foo::<i32> and apply this
    bound generic argument into the impl block. The fn type for this function
    test is:
    
    fn <T,Y>test(a:T, b:Y);
    
    But the impl block has a Self of Foo<T> so we need to inherit the T
    argument from the previous Foo::<i32> which was missing.
    
    Fixes #893

Diff:
---
 gcc/rust/typecheck/rust-hir-type-check-path.cc    | 85 +++++++++++++++++------
 gcc/rust/typecheck/rust-substitution-mapper.cc    | 18 +++++
 gcc/rust/typecheck/rust-substitution-mapper.h     |  3 +
 gcc/rust/typecheck/rust-tyty.cc                   | 39 +++++++++++
 gcc/rust/typecheck/rust-tyty.h                    | 51 ++++++++++++++
 gcc/testsuite/rust/compile/torture/issue-893-2.rs | 35 ++++++++++
 gcc/testsuite/rust/compile/torture/issue-893.rs   | 11 +++
 7 files changed, 219 insertions(+), 23 deletions(-)

diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc
index f00cd16ca73..cd6d67a56ff 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-path.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc
@@ -133,11 +133,6 @@ TypeCheckExpr::visit (HIR::PathInExpression &expr)
 
   size_t offset = -1;
   TyTy::BaseType *tyseg = resolve_root_path (expr, &offset, &resolved_node_id);
-
-  if (tyseg == nullptr)
-    {
-      rust_debug_loc (expr.get_locus (), "failed to resolve root_seg");
-    }
   rust_assert (tyseg != nullptr);
 
   if (tyseg->get_kind () == TyTy::TypeKind::ERROR)
@@ -318,12 +313,12 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 {
   NodeId resolved_node_id = root_resolved_node_id;
   TyTy::BaseType *prev_segment = tyseg;
+  bool reciever_is_generic = prev_segment->get_kind () == TyTy::TypeKind::PARAM;
+
   for (size_t i = offset; i < segments.size (); i++)
     {
       HIR::PathExprSegment &seg = segments.at (i);
 
-      bool reciever_is_generic
-	= prev_segment->get_kind () == TyTy::TypeKind::PARAM;
       bool probe_bounds = true;
       bool probe_impls = !reciever_is_generic;
       bool ignore_mandatory_trait_items = !reciever_is_generic;
@@ -359,6 +354,7 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
       prev_segment = tyseg;
       tyseg = candidate.ty;
 
+      HIR::ImplBlock *associated_impl_block = nullptr;
       if (candidate.is_enum_candidate ())
 	{
 	  const TyTy::VariantDef *variant = candidate.item.enum_field.variant;
@@ -380,6 +376,8 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 	{
 	  resolved_node_id
 	    = candidate.item.impl.impl_item->get_impl_mappings ().get_nodeid ();
+
+	  associated_impl_block = candidate.item.impl.parent;
 	}
       else
 	{
@@ -403,6 +401,52 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 	      // we need a new ty_ref_id for this trait item
 	      tyseg = tyseg->clone ();
 	      tyseg->set_ty_ref (mappings->get_next_hir_id ());
+
+	      // get the associated impl block
+	      associated_impl_block = impl;
+	    }
+	}
+
+      if (associated_impl_block != nullptr)
+	{
+	  // get the type of the parent Self
+	  HirId impl_ty_id
+	    = associated_impl_block->get_type ()->get_mappings ().get_hirid ();
+	  TyTy::BaseType *impl_block_ty = nullptr;
+	  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 (tyseg->needs_generic_substitutions ())
+	{
+	  if (!prev_segment->needs_generic_substitutions ())
+	    {
+	      auto used_args_in_prev_segment
+		= GetUsedSubstArgs::From (prev_segment);
+
+	      if (!used_args_in_prev_segment.is_error ())
+		{
+		  if (SubstMapperInternal::mappings_are_bound (
+			tyseg, used_args_in_prev_segment))
+		    {
+		      tyseg = SubstMapperInternal::Resolve (
+			tyseg, used_args_in_prev_segment);
+		    }
+		}
 	    }
 	}
 
@@ -420,30 +464,25 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 	  if (tyseg->get_kind () == TyTy::TypeKind::ERROR)
 	    return;
 	}
-    }
-
-  context->insert_receiver (expr_mappings.get_hirid (), prev_segment);
-  if (tyseg->needs_generic_substitutions ())
-    {
-      Location locus = segments.back ().get_locus ();
-      if (!prev_segment->needs_generic_substitutions ())
-	{
-	  auto used_args_in_prev_segment
-	    = GetUsedSubstArgs::From (prev_segment);
-	  if (!used_args_in_prev_segment.is_error ())
-	    tyseg
-	      = SubstMapperInternal::Resolve (tyseg, used_args_in_prev_segment);
-	}
-      else
+      else if (tyseg->needs_generic_substitutions () && !reciever_is_generic)
 	{
+	  Location locus = seg.get_locus ();
 	  tyseg = SubstMapper::InferSubst (tyseg, locus);
+	  if (tyseg->get_kind () == TyTy::TypeKind::ERROR)
+	    return;
 	}
+    }
 
+  rust_assert (resolved_node_id != UNKNOWN_NODEID);
+  if (tyseg->needs_generic_substitutions () && !reciever_is_generic)
+    {
+      Location locus = segments.back ().get_locus ();
+      tyseg = SubstMapper::InferSubst (tyseg, locus);
       if (tyseg->get_kind () == TyTy::TypeKind::ERROR)
 	return;
     }
 
-  rust_assert (resolved_node_id != UNKNOWN_NODEID);
+  context->insert_receiver (expr_mappings.get_hirid (), prev_segment);
 
   // lookup if the name resolver was able to canonically resolve this or not
   NodeId path_resolved_id = UNKNOWN_NODEID;
diff --git a/gcc/rust/typecheck/rust-substitution-mapper.cc b/gcc/rust/typecheck/rust-substitution-mapper.cc
index 430eeeb66ca..f96b9b3ee7a 100644
--- a/gcc/rust/typecheck/rust-substitution-mapper.cc
+++ b/gcc/rust/typecheck/rust-substitution-mapper.cc
@@ -43,5 +43,23 @@ SubstMapperInternal::Resolve (TyTy::BaseType *base,
   return mapper.resolved;
 }
 
+bool
+SubstMapperInternal::mappings_are_bound (
+  TyTy::BaseType *tyseg, TyTy::SubstitutionArgumentMappings &mappings)
+{
+  if (tyseg->get_kind () == TyTy::TypeKind::ADT)
+    {
+      TyTy::ADTType *adt = static_cast<TyTy::ADTType *> (tyseg);
+      return adt->are_mappings_bound (mappings);
+    }
+  else if (tyseg->get_kind () == TyTy::TypeKind::FNDEF)
+    {
+      TyTy::FnType *fn = static_cast<TyTy::FnType *> (tyseg);
+      return fn->are_mappings_bound (mappings);
+    }
+
+  return false;
+}
+
 } // namespace Resolver
 } // namespace Rust
diff --git a/gcc/rust/typecheck/rust-substitution-mapper.h b/gcc/rust/typecheck/rust-substitution-mapper.h
index 310524552b1..4ea4762b4b3 100644
--- a/gcc/rust/typecheck/rust-substitution-mapper.h
+++ b/gcc/rust/typecheck/rust-substitution-mapper.h
@@ -156,6 +156,9 @@ public:
   static TyTy::BaseType *Resolve (TyTy::BaseType *base,
 				  TyTy::SubstitutionArgumentMappings &mappings);
 
+  static bool mappings_are_bound (TyTy::BaseType *ty,
+				  TyTy::SubstitutionArgumentMappings &mappings);
+
   void visit (TyTy::FnType &type) override
   {
     TyTy::SubstitutionArgumentMappings adjusted
diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc
index 6a659239639..2dae0134f72 100644
--- a/gcc/rust/typecheck/rust-tyty.cc
+++ b/gcc/rust/typecheck/rust-tyty.cc
@@ -562,6 +562,45 @@ SubstitutionRef::adjust_mappings_for_this (
 				       mappings.get_locus ());
 }
 
+bool
+SubstitutionRef::are_mappings_bound (SubstitutionArgumentMappings &mappings)
+{
+  std::vector<SubstitutionArg> resolved_mappings;
+  for (size_t i = 0; i < substitutions.size (); i++)
+    {
+      auto &subst = substitutions.at (i);
+
+      SubstitutionArg arg = SubstitutionArg::error ();
+      if (mappings.size () == substitutions.size ())
+	{
+	  mappings.get_argument_at (i, &arg);
+	}
+      else
+	{
+	  if (subst.needs_substitution ())
+	    {
+	      // get from passed in mappings
+	      mappings.get_argument_for_symbol (subst.get_param_ty (), &arg);
+	    }
+	  else
+	    {
+	      // we should already have this somewhere
+	      used_arguments.get_argument_for_symbol (subst.get_param_ty (),
+						      &arg);
+	    }
+	}
+
+      bool ok = !arg.is_error ();
+      if (ok)
+	{
+	  SubstitutionArg adjusted (&subst, arg.get_tyty ());
+	  resolved_mappings.push_back (std::move (adjusted));
+	}
+    }
+
+  return !resolved_mappings.empty ();
+}
+
 // this function assumes that the mappings being passed are for the same type as
 // this new substitution reference so ordering matters here
 SubstitutionArgumentMappings
diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h
index 6c9daf73cae..b6f952ecc53 100644
--- a/gcc/rust/typecheck/rust-tyty.h
+++ b/gcc/rust/typecheck/rust-tyty.h
@@ -980,6 +980,57 @@ public:
   SubstitutionArgumentMappings
   adjust_mappings_for_this (SubstitutionArgumentMappings &mappings);
 
+  // Are the mappings here actually bound to this type. For example imagine the
+  // case:
+  //
+  // struct Foo<T>(T);
+  // impl<T> Foo<T> {
+  //   fn test(self) { ... }
+  // }
+  //
+  // In this case we have a generic ADT of Foo and an impl block of a generic T
+  // on Foo for the Self type. When we it comes to path resolution we can have:
+  //
+  // Foo::<i32>::test()
+  //
+  // This means the first segment of Foo::<i32> returns the ADT Foo<i32> not the
+  // Self ADT bound to the T from the impl block. This means when it comes to
+  // the next segment of test which resolves to the function we need to check
+  // wether the arguments in the struct definition of foo can be bound here
+  // before substituting the previous segments type here. This functions acts as
+  // a guard for the solve_mappings_from_receiver_for_self to handle the case
+  // where arguments are not bound. This is important for this next case:
+  //
+  // struct Baz<A, B>(A, B);
+  // impl Baz<i32, f32> {
+  //   fn test<X>(a: X) -> X {
+  //       a
+  //   }
+  // }
+  //
+  // In this case Baz has been already substituted for the impl's Self to become
+  // ADT<i32, f32> so that the function test only has 1 generic argument of X.
+  // The path for this will be:
+  //
+  // Baz::test::<_>(123)
+  //
+  // So the first segment here will be Baz<_, _> to try and infer the arguments
+  // which will be taken from the impl's Self type in this case since it is
+  // already substituted and like the previous case the check to see if we need
+  // to inherit the previous segments generic arguments takes place but the
+  // generic arguments are not bound to this type as they have already been
+  // substituted.
+  //
+  // Its important to remember from the first example the FnType actually looks
+  // like:
+  //
+  // fn <T>test(self :Foo<T>(T))
+  //
+  // As the generic parameters are "bound" to each of the items in the impl
+  // block. So this check is about wether the arguments we have here can
+  // actually be bound to this type.
+  bool are_mappings_bound (SubstitutionArgumentMappings &mappings);
+
   // struct Foo<A, B>(A, B);
   //
   // impl<T> Foo<T, f32>;
diff --git a/gcc/testsuite/rust/compile/torture/issue-893-2.rs b/gcc/testsuite/rust/compile/torture/issue-893-2.rs
new file mode 100644
index 00000000000..88a865d66dc
--- /dev/null
+++ b/gcc/testsuite/rust/compile/torture/issue-893-2.rs
@@ -0,0 +1,35 @@
+// { dg-additional-options "-w" }
+struct Foo<T>(T);
+impl<T> Foo<T> {
+    fn new<Y>(a: T, b: Y) -> Self {
+        Self(a)
+    }
+}
+
+struct Bar<T>(T);
+impl Bar<i32> {
+    fn baz(self) {}
+
+    fn test() -> i32 {
+        123
+    }
+}
+
+struct Baz<A, B>(A, B);
+impl Baz<i32, f32> {
+    fn test<X>(a: X) -> X {
+        a
+    }
+}
+
+pub fn main() {
+    let a = Foo::<i32>::new::<f32>(123, 456f32);
+    let b = Foo::new::<f32>(123, 456f32);
+
+    let c = Bar::<i32>(123);
+    let d = Bar::baz(c);
+
+    let e = Bar::test();
+
+    let f = Baz::test::<bool>(true);
+}
diff --git a/gcc/testsuite/rust/compile/torture/issue-893.rs b/gcc/testsuite/rust/compile/torture/issue-893.rs
new file mode 100644
index 00000000000..d8245f3e0d8
--- /dev/null
+++ b/gcc/testsuite/rust/compile/torture/issue-893.rs
@@ -0,0 +1,11 @@
+// { dg-additional-options "-w" }
+struct Foo<T>(T);
+impl<T> Foo<T> {
+    fn new<Y>(a: T, b: Y) -> Self {
+        Self(a)
+    }
+}
+
+pub fn test() {
+    let a = Foo::<i32>::new::<f32>(123, 456f32);
+}