public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-7618] gccrs: Fix bounds checking to check both sides
@ 2024-01-16 17:45 Arthur Cohen
  0 siblings, 0 replies; only message in thread
From: Arthur Cohen @ 2024-01-16 17:45 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:74db136a7cf7505691d409e01f757a5b86f119c9

commit r14-7618-g74db136a7cf7505691d409e01f757a5b86f119c9
Author: Philip Herron <herron.philip@googlemail.com>
Date:   Thu May 25 17:43:31 2023 +0100

    gccrs: Fix bounds checking to check both sides
    
    We were missing bounds checking for both lhs and rhs directions this is
    important as we might fail checking for all specified bounds properly.
    This is why for #2236 we need the Type parameter T to realise that it
    _cannot_ coerce to the i32 max function directly without any adjustments
    because T has the specified bound of Deref but i32 does not implement
    Deref. This indrectly forces the autoderef cycle to try a deref in order
    to get an i32 which _will_ match the i32 max function in the case we
    pass an &32 as the type parameter T.
    
    Fixes #2236
    
    gcc/rust/ChangeLog:
    
            * typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::resolve_segments): stop if error
            * typecheck/rust-tyty-bounds.cc (TypeBoundsProbe::assemble_sized_builtin): fix sized options
            * typecheck/rust-tyty.cc (BaseType::satisfies_bound): its ok if its an ?T
            (BaseType::bounds_compatible): likewise
            * typecheck/rust-tyty.h: update prototype
            * typecheck/rust-unify.cc (UnifyRules::go): check both sides bounds
    
    gcc/testsuite/ChangeLog:
    
            * rust/execute/torture/issue-2236.rs: New test.
    
    Signed-off-by: Philip Herron <herron.philip@googlemail.com>

Diff:
---
 gcc/rust/typecheck/rust-hir-type-check-path.cc   |  5 +++
 gcc/rust/typecheck/rust-tyty-bounds.cc           |  7 ++---
 gcc/rust/typecheck/rust-tyty.cc                  | 39 +++++++++++++++++-------
 gcc/rust/typecheck/rust-tyty.h                   |  3 +-
 gcc/rust/typecheck/rust-unify.cc                 | 23 +++++++++++---
 gcc/testsuite/rust/execute/torture/issue-2236.rs | 37 ++++++++++++++++++++++
 6 files changed, 93 insertions(+), 21 deletions(-)

diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc
index 9b5c6fee101..037a2d7dc16 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-path.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc
@@ -422,6 +422,11 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
 				     TyTy::TyWithLocation (prev_segment),
 				     TyTy::TyWithLocation (impl_block_ty),
 				     seg.get_locus ());
+	  bool ok = prev_segment->get_kind () != TyTy::TypeKind::ERROR;
+	  if (!ok)
+	    {
+	      return;
+	    }
 
 	  if (found_impl_trait)
 	    {
diff --git a/gcc/rust/typecheck/rust-tyty-bounds.cc b/gcc/rust/typecheck/rust-tyty-bounds.cc
index 947fd06e686..fec80ddc265 100644
--- a/gcc/rust/typecheck/rust-tyty-bounds.cc
+++ b/gcc/rust/typecheck/rust-tyty-bounds.cc
@@ -111,7 +111,6 @@ TypeBoundsProbe::assemble_sized_builtin ()
     case TyTy::REF:
     case TyTy::POINTER:
     case TyTy::PARAM:
-    case TyTy::ARRAY:
     case TyTy::SLICE:
     case TyTy::FNDEF:
     case TyTy::FNPTR:
@@ -123,16 +122,16 @@ TypeBoundsProbe::assemble_sized_builtin ()
     case TyTy::FLOAT:
     case TyTy::USIZE:
     case TyTy::ISIZE:
+    case TyTy::CLOSURE:
+    case TyTy::INFER:
       assemble_builtin_candidate (Analysis::RustLangItem::SIZED);
       break;
 
-      // not-sure about this.... FIXME
-    case TyTy::INFER:
+    case TyTy::ARRAY:
     case TyTy::NEVER:
     case TyTy::PLACEHOLDER:
     case TyTy::PROJECTION:
     case TyTy::DYNAMIC:
-    case TyTy::CLOSURE:
     case TyTy::ERROR:
       break;
     }
diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc
index 5e9af52c47b..143cab42ab5 100644
--- a/gcc/rust/typecheck/rust-tyty.cc
+++ b/gcc/rust/typecheck/rust-tyty.cc
@@ -273,8 +273,11 @@ BaseType::get_locus () const
 
 // FIXME this is missing locus
 bool
-BaseType::satisfies_bound (const TypeBoundPredicate &predicate) const
+BaseType::satisfies_bound (const TypeBoundPredicate &predicate,
+			   bool emit_error) const
 {
+  bool is_infer_var = destructure ()->get_kind () == TyTy::TypeKind::INFER;
+
   const Resolver::TraitReference *query = predicate.get ();
   for (const auto &bound : specified_bounds)
     {
@@ -283,6 +286,9 @@ BaseType::satisfies_bound (const TypeBoundPredicate &predicate) const
 	return true;
     }
 
+  if (is_infer_var)
+    return true;
+
   bool satisfied = false;
   auto probed = Resolver::TypeBoundsProbe::Probe (this);
   for (const auto &b : probed)
@@ -313,6 +319,11 @@ BaseType::satisfies_bound (const TypeBoundPredicate &predicate) const
       const HIR::ImplBlock &impl = *(b.second);
       for (const auto &item : impl.get_impl_items ())
 	{
+	  bool is_associated_type = item->get_impl_item_type ()
+				    == HIR::ImplItem::ImplItemType::TYPE_ALIAS;
+	  if (!is_associated_type)
+	    continue;
+
 	  TyTy::BaseType *impl_item_ty = nullptr;
 	  Analysis::NodeMapping i = item->get_impl_mappings ();
 	  bool query_ok = Resolver::query_type (i.get_hirid (), &impl_item_ty);
@@ -331,15 +342,21 @@ BaseType::satisfies_bound (const TypeBoundPredicate &predicate) const
 	  // compare the types
 	  if (!bound_ty->can_eq (impl_item_ty, false))
 	    {
-	      RichLocation r (mappings->lookup_location (get_ref ()));
-	      r.add_range (predicate.get_locus ());
-	      r.add_range (mappings->lookup_location (i.get_hirid ()));
-
-	      rust_error_at (
-		r, "expected %<%s%> got %<%s%>",
-		bound_ty->destructure ()->get_name ().c_str (),
-		impl_item_ty->destructure ()->get_name ().c_str ());
-	      return false;
+	      if (!impl_item_ty->can_eq (bound_ty, false))
+		{
+		  if (emit_error)
+		    {
+		      RichLocation r (mappings->lookup_location (get_ref ()));
+		      r.add_range (predicate.get_locus ());
+		      r.add_range (mappings->lookup_location (i.get_hirid ()));
+
+		      rust_error_at (
+			r, "expected %<%s%> got %<%s%>",
+			bound_ty->destructure ()->get_name ().c_str (),
+			impl_item_ty->destructure ()->get_name ().c_str ());
+		    }
+		  return false;
+		}
 	    }
 	}
 
@@ -357,7 +374,7 @@ BaseType::bounds_compatible (const BaseType &other, Location locus,
     unsatisfied_bounds;
   for (auto &bound : get_specified_bounds ())
     {
-      if (!other.satisfies_bound (bound))
+      if (!other.satisfies_bound (bound, emit_error))
 	unsatisfied_bounds.push_back (bound);
     }
 
diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h
index a6a373a0c62..45c0545317c 100644
--- a/gcc/rust/typecheck/rust-tyty.h
+++ b/gcc/rust/typecheck/rust-tyty.h
@@ -114,7 +114,8 @@ public:
   //     2. (For functions) have the same signature
   virtual bool is_equal (const BaseType &other) const;
 
-  bool satisfies_bound (const TypeBoundPredicate &predicate) const;
+  bool satisfies_bound (const TypeBoundPredicate &predicate,
+			bool emit_error) const;
   bool bounds_compatible (const BaseType &other, Location locus,
 			  bool emit_error) const;
   void inherit_bounds (const BaseType &other);
diff --git a/gcc/rust/typecheck/rust-unify.cc b/gcc/rust/typecheck/rust-unify.cc
index 027ec555150..dc809e0090f 100644
--- a/gcc/rust/typecheck/rust-unify.cc
+++ b/gcc/rust/typecheck/rust-unify.cc
@@ -151,13 +151,26 @@ UnifyRules::go ()
 	      rtype->debug_str ().c_str ());
 
   // check bounds
-  if (ltype->num_specified_bounds () > 0)
+  bool should_check_bounds = !ltype->is_equal (*rtype);
+  if (should_check_bounds)
     {
-      if (!ltype->bounds_compatible (*rtype, locus, emit_error))
+      if (ltype->num_specified_bounds () > 0)
 	{
-	  // already emitted an error
-	  emit_error = false;
-	  return new TyTy::ErrorType (0);
+	  if (!ltype->bounds_compatible (*rtype, locus, emit_error))
+	    {
+	      // already emitted an error
+	      emit_error = false;
+	      return new TyTy::ErrorType (0);
+	    }
+	}
+      else if (rtype->num_specified_bounds () > 0)
+	{
+	  if (!rtype->bounds_compatible (*ltype, locus, emit_error))
+	    {
+	      // already emitted an error
+	      emit_error = false;
+	      return new TyTy::ErrorType (0);
+	    }
 	}
     }
 
diff --git a/gcc/testsuite/rust/execute/torture/issue-2236.rs b/gcc/testsuite/rust/execute/torture/issue-2236.rs
new file mode 100644
index 00000000000..1edc5a51762
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/issue-2236.rs
@@ -0,0 +1,37 @@
+// { dg-options "-w" }
+mod core {
+    mod ops {
+        #[lang = "deref"]
+        trait Deref {
+            type Target;
+            fn deref(&self) -> &Self::Target;
+        }
+
+        impl<T> Deref for &T {
+            type Target = T;
+
+            fn deref(&self) -> &T {
+                *self
+            }
+        }
+    }
+}
+
+impl i32 {
+    fn max(self, other: i32) -> i32 {
+        if self > other {
+            self
+        } else {
+            other
+        }
+    }
+}
+
+fn foo<T: core::ops::Deref<Target = i32>>(t: T) -> i32 {
+    t.max(2)
+}
+
+fn main() -> i32 {
+    let a: i32 = 1;
+    foo(&a) - 2
+}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-01-16 17:45 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-16 17:45 [gcc r14-7618] gccrs: Fix bounds checking to check both sides Arthur Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).