[gccrs COMMIT] gccrs: Add missing bounds checks on the associated impl

[email protected]
Newsgroups gmane.comp.gcc.rust,gmane.comp.gcc.patches
Message-ID <[email protected]>
From: Philip Herron <[email protected]>

Before this require_b<T: B> (T) only checked for S<Bad> implement B and
it early accepted the

  impl<T: A> B for S<T>

But we need to validate the impl's bound after we bind the arguments being
used on the generic impl. So when we do a call-expr/method-call expr we are
binding to that impl and we need to validate. When we do that we know that
impl T = Bad and we walk the impl generic params T: A apply the subst for
Bad: A where its is not implemented and we fail and return.

Fixes Rust-GCC/gccrs#4678

gcc/rust/ChangeLog:

	* typecheck/rust-hir-trait-reference.h: add emit_errors option
	* typecheck/rust-hir-trait-resolve.cc (AssociatedImplTrait::bind_impl_for_projection): new
	(AssociatedImplTrait::bind_impl_for_bound): call new validate function
	* typecheck/rust-type-util.cc (lookup_associated_impl_block): make this more generic
	* typecheck/rust-tyty-call.cc (validate_call_argument_associated_impl_bounds): new
	(TypeCheckCallExpr::visit): for each call arg validate and bind
	(TypeCheckMethodCallExpr::check): likewise

gcc/testsuite/ChangeLog:

	* rust/compile/issue-4678.rs: New test.

Signed-off-by: Philip Herron <[email protected]>
---
This change was merged into the gccrs repository and is posted here for
upstream visibility and potential drive-by review, as requested by GCC
release managers.
Each commit email contains a link to its details on github from where you can
find the Pull-Request and associated discussions.


Commit on github: https://github.com/Rust-GCC/gccrs/commit/fde042ef7c7241b40e12b3dabaea90d4298c6fa5

The commit has been mentioned in the following issue(s):
 - Rust-GCC/gccrs#4678: https://github.com/Rust-GCC/gccrs/issues/4678

The commit has been mentioned in the following pull-request(s):
 - https://github.com/Rust-GCC/gccrs/pull/4787

 gcc/rust/typecheck/rust-hir-trait-reference.h |  3 +-
 gcc/rust/typecheck/rust-hir-trait-resolve.cc  | 93 ++++++++++++++++++-
 gcc/rust/typecheck/rust-type-util.cc          |  2 +-
 gcc/rust/typecheck/rust-tyty-call.cc          | 46 +++++++++
 gcc/testsuite/rust/compile/issue-4678.rs      | 23 +++++
 5 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/rust/compile/issue-4678.rs

diff --git a/gcc/rust/typecheck/rust-hir-trait-reference.h b/gcc/rust/typecheck/rust-hir-trait-reference.h
index a8f923826..b26ba0ad5 100644
--- a/gcc/rust/typecheck/rust-hir-trait-reference.h
+++ b/gcc/rust/typecheck/rust-hir-trait-reference.h
@@ -253,7 +253,8 @@ public:
 
   TyTy::SubstitutionArgumentMappings
   bind_impl_for_bound (TyTy::BaseType *receiver,
-		       const TyTy::TypeBoundPredicate &bound, location_t locus);
+		       const TyTy::TypeBoundPredicate &bound, location_t locus,
+		       bool emit_error = false);
 
 private:
   TraitReference *trait;
diff --git a/gcc/rust/typecheck/rust-hir-trait-resolve.cc b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
index b036c634f..3ed46e0f0 100644
--- a/gcc/rust/typecheck/rust-hir-trait-resolve.cc
+++ b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
@@ -17,16 +17,100 @@
 // <http://www.gnu.org/licenses/>.
 
 #include "rust-hir-trait-resolve.h"
+#include "rich-location.h"
 #include "rust-hir-trait-reference.h"
 #include "rust-hir-type-check-expr.h"
 #include "rust-rib.h"
 #include "rust-substitution-mapper.h"
+#include "text-range-label.h"
 #include "rust-type-util.h"
 #include "rust-finalized-name-resolution-context.h"
 
 namespace Rust {
 namespace Resolver {
 
+static bool
+validate_impl_substitution_bounds (
+  const std::vector<TyTy::SubstitutionArg> &resolved_args, location_t locus,
+  bool emit_error)
+{
+  auto &mctx = Analysis::Mappings::get ();
+
+  std::vector<TyTy::SubstitutionArg> args;
+  for (const auto &arg : resolved_args)
+    args.push_back (arg);
+
+  TyTy::SubstitutionArgumentMappings mappings (std::move (args),
+					       {} /*binding_args*/,
+					       TyTy::RegionParamList (0),
+					       locus);
+
+  for (const auto &arg : resolved_args)
+    {
+      TyTy::BaseGeneric *param
+	= const_cast<TyTy::BaseGeneric *> (arg.get_param_ty ());
+      if (param == nullptr)
+	continue;
+
+      TyTy::BaseType *resolved_arg = arg.get_tyty ();
+      if (resolved_arg->get_kind () == TyTy::TypeKind::PARAM)
+	resolved_arg
+	  = static_cast<TyTy::ParamType *> (resolved_arg)->resolve ();
+
+      if (resolved_arg->get_kind () == TyTy::TypeKind::PARAM
+	  || resolved_arg->get_kind () == TyTy::TypeKind::INFER)
+	continue;
+
+      auto arg_type_locus
+	= mctx.lookup_location (arg.get_tyty ()->get_ty_ref ());
+      for (auto bound : param->get_specified_bounds ())
+	{
+	  auto bound_locus = bound.get_locus ();
+	  auto trait_locus = bound.get ()->get_locus ();
+	  bound.apply_argument_mappings (mappings, false /*is_super_trait*/);
+
+	  if (!resolved_arg->satisfies_bound (bound, false /*emit_error*/))
+	    {
+	      if (emit_error)
+		{
+		  rich_location r (line_table, locus);
+
+		  std::string arg_label_text = "the trait " + bound.get_name ()
+					       + " is not implemented for "
+					       + resolved_arg->get_name ();
+
+		  text_range_label arg_label (arg_label_text.c_str ());
+		  r.add_range (arg_type_locus, SHOW_RANGE_WITHOUT_CARET,
+			       &arg_label);
+
+		  bool ambiguous = false;
+		  auto *trait_impl
+		    = lookup_associated_impl_block (bound, resolved_arg,
+						    &ambiguous);
+		  text_range_label trait_label (
+		    "this trait has no implementations, consider adding one");
+		  if (trait_impl == nullptr)
+		    r.add_range (trait_locus, SHOW_RANGE_WITHOUT_CARET,
+				 &trait_label);
+
+		  text_range_label bound_label (
+		    "unsatisfied trait bound introduced here");
+		  r.add_range (bound_locus, SHOW_RANGE_WITHOUT_CARET,
+			       &bound_label);
+
+		  rust_error_at (r, ErrorCode::E0277,
+				 "the trait bound %<%s: %s%> is not satisfied",
+				 resolved_arg->get_name ().c_str (),
+				 bound.get_name ().c_str ());
+		}
+	      return false;
+	    }
+	}
+    }
+
+  return true;
+}
+
 TraitItemReference
 ResolveTraitItemToRef::Resolve (
   HIR::TraitItem &item, TyTy::BaseType *self,
@@ -598,6 +682,10 @@ AssociatedImplTrait::bind_impl_for_projection (TyTy::ProjectionType &proj,
       resolved_args.emplace_back (&p, r);
     }
 
+  if (!validate_impl_substitution_bounds (resolved_args, locus,
+					  false /*emit_error*/))
+    return TyTy::SubstitutionArgumentMappings::error ();
+
   return TyTy::SubstitutionArgumentMappings (std::move (resolved_args),
 					     {} /*binding_args*/,
 					     TyTy::RegionParamList (0)
@@ -608,7 +696,7 @@ AssociatedImplTrait::bind_impl_for_projection (TyTy::ProjectionType &proj,
 TyTy::SubstitutionArgumentMappings
 AssociatedImplTrait::bind_impl_for_bound (TyTy::BaseType *receiver,
 					  const TyTy::TypeBoundPredicate &bound,
-					  location_t locus)
+					  location_t locus, bool emit_error)
 {
   // Same shape as bind_impl_for_projection but the receiver/trait-args are
   // taken from the (binding, bound) pair instead of a ProjectionType.
@@ -711,6 +799,9 @@ AssociatedImplTrait::bind_impl_for_bound (TyTy::BaseType *receiver,
       resolved_args.emplace_back (&p, r);
     }
 
+  if (!validate_impl_substitution_bounds (resolved_args, locus, emit_error))
+    return TyTy::SubstitutionArgumentMappings::error ();
+
   return TyTy::SubstitutionArgumentMappings (std::move (resolved_args),
 					     {} /*binding_args*/,
 					     TyTy::RegionParamList (0)
diff --git a/gcc/rust/typecheck/rust-type-util.cc b/gcc/rust/typecheck/rust-type-util.cc
index 5fcea2c8b..5f43bbdbe 100644
--- a/gcc/rust/typecheck/rust-type-util.cc
+++ b/gcc/rust/typecheck/rust-type-util.cc
@@ -395,7 +395,7 @@ lookup_associated_impl_block (const TyTy::TypeBoundPredicate &bound,
       if (found_impl_trait)
 	{
 	  // compare the bounds from here i think is what we can do:
-	  if (bound.is_equal (associated->get_predicate ()))
+	  if (bound.get ()->is_equal (*associated->get_predicate ().get ()))
 	    {
 	      associated_impl_traits.push_back (associated);
 	    }
diff --git a/gcc/rust/typecheck/rust-tyty-call.cc b/gcc/rust/typecheck/rust-tyty-call.cc
index 5a2b105e2..7ec23281a 100644
--- a/gcc/rust/typecheck/rust-tyty-call.cc
+++ b/gcc/rust/typecheck/rust-tyty-call.cc
@@ -20,6 +20,7 @@
 #include "rust-hir-type-check-expr.h"
 #include "rust-hir-type-check.h"
 #include "rust-type-util.h"
+#include "rust-hir-trait-reference.h"
 
 namespace Rust {
 namespace TyTy {
@@ -53,6 +54,43 @@ emit_unexpected_argument_error (location_t loc,
 		 unexpected_arg_count);
 }
 
+static bool
+validate_call_argument_associated_impl_bounds (BaseType *param_ty,
+					       BaseType *argument_ty,
+					       location_t locus)
+{
+  auto *context = Resolver::TypeCheckContext::get ();
+
+  // impl bodies are checked generically
+  if (context->have_function_context ()
+      && context->peek_context ().get_type ()
+	   == Resolver::TypeCheckContextItem::IMPL_ITEM)
+    return true;
+
+  auto *resolved_argument_ty = argument_ty->destructure ();
+  if (resolved_argument_ty->get_kind () == TypeKind::PARAM
+      || resolved_argument_ty->get_kind () == TypeKind::INFER
+      || resolved_argument_ty->get_kind () == TypeKind::PROJECTION)
+    return true;
+
+  for (auto bound : param_ty->get_specified_bounds ())
+    {
+      bool ambigious = false;
+      auto associated
+	= Resolver::lookup_associated_impl_block (bound, argument_ty,
+						  &ambigious);
+      if (associated == nullptr)
+	continue;
+
+      auto mapping = associated->bind_impl_for_bound (argument_ty, bound, locus,
+						      true /*emit_error*/);
+      if (mapping.is_error ())
+	return false;
+    }
+
+  return true;
+}
+
 void
 TypeCheckCallExpr::visit (ADTType &type)
 {
@@ -192,6 +230,10 @@ TypeCheckCallExpr::visit (FnType &type)
 	    {
 	      return;
 	    }
+
+	  if (!validate_call_argument_associated_impl_bounds (
+		param_ty, argument_expr_tyty, argument->get_locus ()))
+	    return;
 	}
       else
 	{
@@ -420,6 +462,10 @@ TypeCheckMethodCallExpr::check (FnType &type)
 	  return new ErrorType (type.get_ref ());
 	}
 
+      if (!validate_call_argument_associated_impl_bounds (
+	    param_ty, argument_expr_tyty, argument.get_locus ()))
+	return new ErrorType (type.get_ref ());
+
       i++;
     }
 
diff --git a/gcc/testsuite/rust/compile/issue-4678.rs b/gcc/testsuite/rust/compile/issue-4678.rs
new file mode 100644
index 000000000..a088edb80
--- /dev/null
+++ b/gcc/testsuite/rust/compile/issue-4678.rs
@@ -0,0 +1,23 @@
+#![feature(no_core)]
+#![no_core]
+#![feature(lang_items)]
+
+#[lang = "sized"]
+trait Sized {}
+
+trait A {}
+trait B {}
+
+struct S<T>(T);
+
+impl<T: A> B for S<T> {}
+
+struct Bad;
+
+fn require_b<T: B>(_: T) {}
+
+fn main() {
+    let b = S(Bad);
+    require_b(b);
+    // { dg-error "the trait bound .Bad: A. is not satisfied .E0277." "" { target *-*-* } .-1 }
+}

base-commit: 79c81d0527fe0527084fc831c5ecebe5a39d30c3
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.