Merge #1030

1030: Rewrite our unconstrained type-param error checking r=philberty a=philberty

This is a series of patches that were all required to fix this issue. We
now take advantage of our substitutions abstractions and traits
so that our TypeBoundPredicate's which form the basis of our HRTB code
I think this class is almost akin to rustc existential-trait-references. This now
reuses the same code path to give us the same error checking for generics
as we get with ADT's, functions etc.

With this refactoring in place we can then reuse the abstractions to map the
ID's from the used arguments in the type-bound-predicate, the impl block type
substation mappings and the self type itself.

There are quite a few cases to handle and our testsuite picked up all the regressions
so no behaviour of our existing test-cases have changed now. See each commit for
more detailed information.

Fixes #1019 
Addresses #849

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

gcc/rust/Make-lang.in

@@ -99,6 +99,7 @@ GRS_OBJS = \
     rust/rust-hir-type-check-pattern.o \
     rust/rust-hir-type-check-expr.o \
     rust/rust-hir-dot-operator.o \
+    rust/rust-hir-type-check-base.o \
     rust/rust-autoderef.o \
     rust/rust-substitution-mapper.o \
     rust/rust-lint-marklive.o \

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

@@ -133,8 +133,7 @@ struct GenericArgs
 
   GenericArgs (std::vector<Lifetime> lifetime_args,
 	       std::vector<std::unique_ptr<Type> > type_args,
-	       std::vector<GenericArgsBinding> binding_args,
-	       Location locus = Location ())
+	       std::vector<GenericArgsBinding> binding_args, Location locus)
     : lifetime_args (std::move (lifetime_args)),
       type_args (std::move (type_args)),
       binding_args (std::move (binding_args)), locus (locus)
@@ -471,7 +470,7 @@ class TypePathSegmentGeneric : public TypePathSegment
 		       has_separating_scope_resolution, locus),
       generic_args (GenericArgs (std::move (lifetime_args),
 				 std::move (type_args),
-				 std::move (binding_args)))
+				 std::move (binding_args), locus))
   {}
 
   std::string as_string () const override;

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

@@ -181,27 +181,48 @@ class TraitReference
 public:
   TraitReference (const HIR::Trait *hir_trait_ref,
 		  std::vector<TraitItemReference> item_refs,
-		  std::vector<const TraitReference *> super_traits)
+		  std::vector<const TraitReference *> super_traits,
+		  std::vector<TyTy::SubstitutionParamMapping> substs)
     : hir_trait_ref (hir_trait_ref), item_refs (item_refs),
       super_traits (super_traits)
-  {}
+  {
+    trait_substs.clear ();
+    trait_substs.reserve (substs.size ());
+    for (const auto &p : substs)
+      trait_substs.push_back (p.clone ());
+  }
 
   TraitReference (TraitReference const &other)
-    : hir_trait_ref (other.hir_trait_ref), item_refs (other.item_refs)
-  {}
+    : hir_trait_ref (other.hir_trait_ref), item_refs (other.item_refs),
+      super_traits (other.super_traits)
+  {
+    trait_substs.clear ();
+    trait_substs.reserve (other.trait_substs.size ());
+    for (const auto &p : other.trait_substs)
+      trait_substs.push_back (p.clone ());
+  }
 
   TraitReference &operator= (TraitReference const &other)
   {
     hir_trait_ref = other.hir_trait_ref;
     item_refs = other.item_refs;
+    super_traits = other.super_traits;
+
+    trait_substs.clear ();
+    trait_substs.reserve (other.trait_substs.size ());
+    for (const auto &p : other.trait_substs)
+      trait_substs.push_back (p.clone ());
 
     return *this;
   }
 
   TraitReference (TraitReference &&other) = default;
   TraitReference &operator= (TraitReference &&other) = default;
 
-  static TraitReference error () { return TraitReference (nullptr, {}, {}); }
+  static TraitReference error ()
+  {
+    return TraitReference (nullptr, {}, {}, {});
+  }
 
   bool is_error () const { return hir_trait_ref == nullptr; }
 
@@ -384,10 +405,18 @@ class TraitReference
     return is_safe;
   }
 
+  bool trait_has_generics () const { return !trait_substs.empty (); }
+
+  std::vector<TyTy::SubstitutionParamMapping> get_trait_substs () const
+  {
+    return trait_substs;
+  }
+
 private:
   const HIR::Trait *hir_trait_ref;
   std::vector<TraitItemReference> item_refs;
   std::vector<const TraitReference *> super_traits;
+  std::vector<TyTy::SubstitutionParamMapping> trait_substs;
 };
 
 class AssociatedImplTrait

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

@@ -150,8 +150,13 @@ class TraitResolver : public TypeCheckBase
 
     // They also inherit themselves as a bound this enables a trait item to
     // reference other Self::trait_items
+    std::vector<TyTy::SubstitutionParamMapping> self_subst_copy;
+    for (auto &sub : substitutions)
+      self_subst_copy.push_back (sub.clone ());
+
     specified_bounds.push_back (
       TyTy::TypeBoundPredicate (trait_reference->get_mappings ().get_defid (),
+				std::move (self_subst_copy),
 				trait_reference->get_locus ()));
 
     std::vector<const TraitReference *> super_traits;
@@ -168,8 +173,8 @@ class TraitResolver : public TypeCheckBase
 		// FIXME this might be recursive we need a check for that
 
 		TraitReference *trait = resolve_trait_path (b->get_path ());
-		TyTy::TypeBoundPredicate predicate (
-		  trait->get_mappings ().get_defid (), bound->get_locus ());
+		TyTy::TypeBoundPredicate predicate (*trait,
+						    bound->get_locus ());
 
 		specified_bounds.push_back (std::move (predicate));
 		super_traits.push_back (predicate.get ());
@@ -193,7 +198,8 @@ class TraitResolver : public TypeCheckBase
       }
 
     TraitReference trait_object (trait_reference, item_refs,
-				 std::move (super_traits));
+				 std::move (super_traits),
+				 std::move (substitutions));
     context->insert_trait_reference (
       trait_reference->get_mappings ().get_defid (), std::move (trait_object));
 

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

@@ -0,0 +1,84 @@
+// Copyright (C) 2020-2022 Free Software Foundation, Inc.
+
+// This file is part of GCC.
+
+// GCC is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3, or (at your option) any later
+// version.
+
+// GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+// WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+// for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with GCC; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include "rust-hir-type-check-base.h"
+
+namespace Rust {
+namespace Resolver {
+
+bool
+TypeCheckBase::check_for_unconstrained (
+  const std::vector<TyTy::SubstitutionParamMapping> &params_to_constrain,
+  const TyTy::SubstitutionArgumentMappings &constraint_a,
+  const TyTy::SubstitutionArgumentMappings &constraint_b,
+  const TyTy::BaseType *reference)
+{
+  std::set<HirId> symbols_to_constrain;
+  std::map<HirId, Location> symbol_to_location;
+  for (const auto &p : params_to_constrain)
+    {
+      HirId ref = p.get_param_ty ()->get_ref ();
+      symbols_to_constrain.insert (ref);
+      symbol_to_location.insert ({ref, p.get_param_locus ()});
+    }
+
+  // set up the set of constrained symbols
+  std::set<HirId> constrained_symbols;
+  for (const auto &c : constraint_a.get_mappings ())
+    {
+      const TyTy::BaseType *arg = c.get_tyty ();
+      if (arg != nullptr)
+	{
+	  const TyTy::BaseType *p = arg->get_root ();
+	  constrained_symbols.insert (p->get_ty_ref ());
+	}
+    }
+  for (const auto &c : constraint_b.get_mappings ())
+    {
+      const TyTy::BaseType *arg = c.get_tyty ();
+      if (arg != nullptr)
+	{
+	  const TyTy::BaseType *p = arg->get_root ();
+	  constrained_symbols.insert (p->get_ty_ref ());
+	}
+    }
+
+  const auto root = reference->get_root ();
+  if (root->get_kind () == TyTy::TypeKind::PARAM)
+    {
+      const TyTy::ParamType *p = static_cast<const TyTy::ParamType *> (root);
+      constrained_symbols.insert (p->get_ty_ref ());
+    }
+
+  // check for unconstrained
+  bool unconstrained = false;
+  for (auto &sym : symbols_to_constrain)
+    {
+      bool used = constrained_symbols.find (sym) != constrained_symbols.end ();
+      if (!used)
+	{
+	  Location locus = symbol_to_location.at (sym);
+	  rust_error_at (locus, "unconstrained type parameter");
+	  unconstrained = true;
+	}
+    }
+  return unconstrained;
+}
+
+} // namespace Resolver
+} // namespace Rust

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

@@ -47,6 +47,14 @@ class TypeCheckBase : public HIR::HIRFullVisitorBase
 
   TraitReference *resolve_trait_path (HIR::TypePath &);
 
+  TyTy::TypeBoundPredicate get_predicate_from_bound (HIR::TypePath &path);
+
+  bool check_for_unconstrained (
+    const std::vector<TyTy::SubstitutionParamMapping> &params_to_constrain,
+    const TyTy::SubstitutionArgumentMappings &constraint_a,
+    const TyTy::SubstitutionArgumentMappings &constraint_b,
+    const TyTy::BaseType *reference);
+
   Analysis::Mappings *mappings;
   Resolver *resolver;
   TypeCheckContext *context;

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

@@ -70,34 +70,15 @@ class TypeCheckItem : public TypeCheckBase
 	  }
       }
 
-    std::vector<TyTy::TypeBoundPredicate> specified_bounds;
+    auto specified_bound = TyTy::TypeBoundPredicate::error ();
     TraitReference *trait_reference = &TraitReference::error_node ();
     if (impl_block.has_trait_ref ())
       {
 	std::unique_ptr<HIR::TypePath> &ref = impl_block.get_trait_ref ();
 	trait_reference = TraitResolver::Resolve (*ref.get ());
 	rust_assert (!trait_reference->is_error ());
 
-	// setup the bound
-	TyTy::TypeBoundPredicate predicate (
-	  trait_reference->get_mappings ().get_defid (), ref->get_locus ());
-	auto &final_seg = ref->get_final_segment ();
-	if (final_seg->is_generic_segment ())
-	  {
-	    auto final_generic_seg
-	      = static_cast<HIR::TypePathSegmentGeneric *> (final_seg.get ());
-	    if (final_generic_seg->has_generic_args ())
-	      {
-		HIR::GenericArgs &generic_args
-		  = final_generic_seg->get_generic_args ();
-
-		// this is applying generic arguments to a trait
-		// reference
-		predicate.apply_generic_arguments (&generic_args);
-	      }
-	  }
-
-	specified_bounds.push_back (std::move (predicate));
+	specified_bound = get_predicate_from_bound (*ref.get ());
       }
 
     TyTy::BaseType *self = nullptr;
@@ -108,11 +89,25 @@ class TypeCheckItem : public TypeCheckBase
 		       "failed to resolve Self for ImplBlock");
 	return;
       }
-    // inherit the bounds
-    self->inherit_bounds (specified_bounds);
 
+    // inherit the bounds
+    if (!specified_bound.is_error ())
+      self->inherit_bounds ({specified_bound});
+
+    // check for any unconstrained type-params
+    const TyTy::SubstitutionArgumentMappings trait_constraints
+      = specified_bound.get_substitution_arguments ();
+    const TyTy::SubstitutionArgumentMappings impl_constraints
+      = GetUsedSubstArgs::From (self);
+
+    bool impl_block_has_unconstrained_typarams
+      = check_for_unconstrained (substitutions, trait_constraints,
+				 impl_constraints, self);
+    if (impl_block_has_unconstrained_typarams)
+      return;
+
+    // validate the impl items
     bool is_trait_impl_block = !trait_reference->is_error ();
-
     std::vector<const TraitItemReference *> trait_item_refs;
     for (auto &impl_item : impl_block.get_impl_items ())
       {

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

@@ -468,9 +468,8 @@ class TypeCheckTopLevel : public TypeCheckBase
 	ResolveWhereClauseItem::Resolve (*where_clause_item.get ());
       }
 
-    auto self
-      = TypeCheckType::Resolve (impl_block.get_type ().get (), &substitutions);
-    if (self == nullptr || self->get_kind () == TyTy::TypeKind::ERROR)
+    auto self = TypeCheckType::Resolve (impl_block.get_type ().get ());
+    if (self->get_kind () == TyTy::TypeKind::ERROR)
       return;
 
     for (auto &impl_item : impl_block.get_impl_items ())

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

@@ -84,11 +84,6 @@ TypeCheckType::visit (HIR::TypePath &path)
 	}
 
       translated = SubstMapper::Resolve (path_type, path.get_locus (), &args);
-      if (translated->get_kind () != TyTy::TypeKind::ERROR
-	  && mappings != nullptr)
-	{
-	  check_for_unconstrained (args.get_type_args ());
-	}
     }
   else if (!args.is_empty ())
     {
@@ -548,27 +543,11 @@ TypeCheckType::visit (HIR::TraitObjectType &type)
       HIR::TypeParamBound &b = *bound.get ();
       HIR::TraitBound &trait_bound = static_cast<HIR::TraitBound &> (b);
 
-      auto &type_path = trait_bound.get_path ();
-      TraitReference *trait = resolve_trait_path (type_path);
-      TyTy::TypeBoundPredicate predicate (trait->get_mappings ().get_defid (),
-					  trait_bound.get_locus ());
-      auto &final_seg = type_path.get_final_segment ();
-      if (final_seg->is_generic_segment ())
-	{
-	  auto final_generic_seg
-	    = static_cast<HIR::TypePathSegmentGeneric *> (final_seg.get ());
-	  if (final_generic_seg->has_generic_args ())
-	    {
-	      HIR::GenericArgs &generic_args
-		= final_generic_seg->get_generic_args ();
-
-	      // this is applying generic arguments to a trait
-	      // reference
-	      predicate.apply_generic_arguments (&generic_args);
-	    }
-	}
+      TyTy::TypeBoundPredicate predicate
+	= get_predicate_from_bound (trait_bound.get_path ());
 
-      if (predicate.is_object_safe (true, type.get_locus ()))
+      if (!predicate.is_error ()
+	  && predicate.is_object_safe (true, type.get_locus ()))
 	specified_bounds.push_back (std::move (predicate));
     }