Fix unconstrained type parameter checks

This patch removes our old method of checking for unconstrained type
parameters which only worked for the basic cases such as:

  impl<T> Foo { }

But checking for unconstrained types is more complex, we need to handle
covariant types such as:

  impl<T> *T { }

Or

  struct foo<X,Y>(X,Y);
  impl<T> foo<&T,*T> {}

This rewrites the algorithm to take advantage of our substition
abstractions and HirIds so we can map the ids of the type-params to be
constrained and look at the trait-references used-arguments when the
generics are applied (or they may be empty) and then do the same for any
used arguments on an algebraic data type.

Fixes #1019

Author: philberty

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/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,
-					    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,26 +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, 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));
     }
 

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

@@ -57,12 +57,9 @@ class TypeCheckType : public TypeCheckBase
   using Rust::Resolver::TypeCheckBase::visit;
 
 public:
-  static TyTy::BaseType *
-  Resolve (HIR::Type *type,
-	   std::vector<TyTy::SubstitutionParamMapping> *subst_mappings
-	   = nullptr)
+  static TyTy::BaseType *Resolve (HIR::Type *type)
   {
-    TypeCheckType resolver (type->get_mappings ().get_hirid (), subst_mappings);
+    TypeCheckType resolver (type->get_mappings ().get_hirid ());
     type->accept_vis (resolver);
     rust_assert (resolver.translated != nullptr);
     resolver.context->insert_type (type->get_mappings (), resolver.translated);
@@ -159,43 +156,10 @@ class TypeCheckType : public TypeCheckBase
   void visit (HIR::TraitObjectType &type) override;
 
 private:
-  TypeCheckType (HirId id,
-		 std::vector<TyTy::SubstitutionParamMapping> *subst_mappings)
-    : TypeCheckBase (), subst_mappings (subst_mappings),
-      translated (new TyTy::ErrorType (id))
+  TypeCheckType (HirId id)
+    : TypeCheckBase (), translated (new TyTy::ErrorType (id))
   {}
 
-  void
-  check_for_unconstrained (std::vector<std::unique_ptr<HIR::Type>> &type_args)
-  {
-    std::map<std::string, Location> param_location_map;
-    std::set<std::string> param_tys;
-
-    if (subst_mappings != nullptr)
-      {
-	for (auto &mapping : *subst_mappings)
-	  {
-	    std::string sym = mapping.get_param_ty ()->get_symbol ();
-	    param_tys.insert (sym);
-	    param_location_map[sym] = mapping.get_generic_param ().get_locus ();
-	  }
-      }
-
-    std::set<std::string> args;
-    for (auto &arg : type_args)
-      args.insert (arg->as_string ());
-
-    for (auto &exp : param_tys)
-      {
-	bool used = args.find (exp) != args.end ();
-	if (!used)
-	  {
-	    Location locus = param_location_map.at (exp);
-	    rust_error_at (locus, "unconstrained type parameter");
-	  }
-      }
-  }
-
   TyTy::BaseType *resolve_root_path (HIR::TypePath &path, size_t *offset,
 				     NodeId *root_resolved_node_id);
 
@@ -205,7 +169,6 @@ class TypeCheckType : public TypeCheckBase
     TyTy::BaseType *tyseg, const Analysis::NodeMapping &expr_mappings,
     Location expr_locus);
 
-  std::vector<TyTy::SubstitutionParamMapping> *subst_mappings;
   TyTy::BaseType *translated;
 };
 
@@ -245,28 +208,10 @@ class TypeResolveGenericParam : public TypeCheckBase
 		  HIR::TraitBound *b
 		    = static_cast<HIR::TraitBound *> (bound.get ());
 
-		  auto &type_path = b->get_path ();
-		  TraitReference *trait = resolve_trait_path (type_path);
-		  TyTy::TypeBoundPredicate predicate (*trait, b->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);
-			}
-		    }
-
-		  specified_bounds.push_back (std::move (predicate));
+		  TyTy::TypeBoundPredicate predicate
+		    = get_predicate_from_bound (b->get_path ());
+		  if (!predicate.is_error ())
+		    specified_bounds.push_back (std::move (predicate));
 		}
 		break;
 
@@ -315,28 +260,10 @@ class ResolveWhereClauseItem : public TypeCheckBase
 	      HIR::TraitBound *b
 		= static_cast<HIR::TraitBound *> (bound.get ());
 
-	      auto &type_path = b->get_path ();
-	      TraitReference *trait = resolve_trait_path (type_path);
-	      TyTy::TypeBoundPredicate predicate (*trait, b->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);
-		    }
-		}
-
-	      specified_bounds.push_back (std::move (predicate));
+	      TyTy::TypeBoundPredicate predicate
+		= get_predicate_from_bound (b->get_path ());
+	      if (!predicate.is_error ())
+		specified_bounds.push_back (std::move (predicate));
 	    }
 	    break;