]> git.ipfire.org Git - people/ms/gcc.git/commitdiff
gccrs: Fix method resolution to use TryCoerce
authorPhilip Herron <herron.philip@googlemail.com>
Mon, 27 Feb 2023 19:16:00 +0000 (19:16 +0000)
committerPhilip Herron <philip.herron@embecosm.com>
Tue, 28 Feb 2023 20:38:35 +0000 (20:38 +0000)
Rust allows us to call generic pointer methods on pointers so in non
generic contexts the old code using the bad can_eq interface couldn't
handle this case. So taking advantage of our new unify_and interface to try
and infer when required we can start using our TryCoerce interface to reuse
existing code to assemble possible candidates more acurately using the
existing coercion rules.

Fixes #1901 #878

Signed-off-by: Philip Herron <herron.philip@googlemail.com>
gcc/rust/ChangeLog:

* typecheck/rust-coercion.cc (TypeCoercionRules::Coerce): Add new try_flag
(TypeCoercionRules::TypeCoercionRules): set new try flag
(TypeCoercionRules::do_coercion): default to a final unify_and in the else case
(TypeCoercionRules::coerce_unsafe_ptr): cannot coerce to a ptr from ref during autoderef
(TypeCoercionRules::coerce_borrowed_pointer): respect coerceable mutability
* typecheck/rust-coercion.h: update header
* typecheck/rust-hir-dot-operator.cc (MethodResolver::select): use new TryCoerce interface
(MethodResolver::append_adjustments): ensure we maintain adjustment mappings
* typecheck/rust-hir-dot-operator.h: add new method append_adjustments
* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): extra logging

gcc/testsuite/ChangeLog:

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

gcc/rust/typecheck/rust-coercion.cc
gcc/rust/typecheck/rust-coercion.h
gcc/rust/typecheck/rust-hir-dot-operator.cc
gcc/rust/typecheck/rust-hir-dot-operator.h
gcc/rust/typecheck/rust-hir-type-check-expr.cc
gcc/testsuite/rust/compile/issue-1901.rs [new file with mode: 0644]

index c07ee733514ea30543d6302c673ac8d22ce34731..9831e77cdd084f447fbeba6649028f084b9433ff 100644 (file)
@@ -27,7 +27,7 @@ TypeCoercionRules::CoercionResult
 TypeCoercionRules::Coerce (TyTy::BaseType *receiver, TyTy::BaseType *expected,
                           Location locus, bool allow_autoderef)
 {
-  TypeCoercionRules resolver (expected, locus, true, allow_autoderef);
+  TypeCoercionRules resolver (expected, locus, true, allow_autoderef, false);
   bool ok = resolver.do_coercion (receiver);
   return ok ? resolver.try_result : CoercionResult::get_error ();
 }
@@ -37,16 +37,18 @@ TypeCoercionRules::TryCoerce (TyTy::BaseType *receiver,
                              TyTy::BaseType *expected, Location locus,
                              bool allow_autoderef)
 {
-  TypeCoercionRules resolver (expected, locus, false, allow_autoderef);
+  TypeCoercionRules resolver (expected, locus, false, allow_autoderef, true);
   bool ok = resolver.do_coercion (receiver);
   return ok ? resolver.try_result : CoercionResult::get_error ();
 }
 
 TypeCoercionRules::TypeCoercionRules (TyTy::BaseType *expected, Location locus,
-                                     bool emit_errors, bool allow_autoderef)
+                                     bool emit_errors, bool allow_autoderef,
+                                     bool try_flag)
   : AutoderefCycle (!allow_autoderef), mappings (Analysis::Mappings::get ()),
     context (TypeCheckContext::get ()), expected (expected), locus (locus),
-    try_result (CoercionResult::get_error ()), emit_errors (emit_errors)
+    try_result (CoercionResult::get_error ()), emit_errors (emit_errors),
+    try_flag (try_flag)
 {}
 
 bool
@@ -139,6 +141,31 @@ TypeCoercionRules::do_coercion (TyTy::BaseType *receiver)
       break;
     }
 
+  // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/coercion.rs#L210
+  switch (receiver->get_kind ())
+    {
+      default: {
+       rust_debug (
+         "do_coercion default unify and infer expected: %s receiver %s",
+         receiver->debug_str ().c_str (), expected->debug_str ().c_str ());
+       TyTy::BaseType *result
+         = unify_site_and (receiver->get_ref (),
+                           TyTy::TyWithLocation (expected),
+                           TyTy::TyWithLocation (receiver),
+                           locus /*unify_locus*/, false /*emit_errors*/,
+                           !try_flag /*commit_if_ok*/, true /*infer*/,
+                           try_flag /*cleanup on error*/);
+       rust_debug ("result");
+       result->debug ();
+       if (result->get_kind () != TyTy::TypeKind::ERROR)
+         {
+           try_result = CoercionResult{{}, result};
+           return true;
+         }
+      }
+      break;
+    }
+
   return !try_result.is_error ();
 }
 
@@ -170,6 +197,7 @@ TypeCoercionRules::coerce_unsafe_ptr (TyTy::BaseType *receiver,
       break;
 
       default: {
+       // FIXME this can probably turn into a unify_and
        if (receiver->can_eq (expected, false))
          return CoercionResult{{}, expected->clone ()};
 
@@ -177,6 +205,13 @@ TypeCoercionRules::coerce_unsafe_ptr (TyTy::BaseType *receiver,
       }
     }
 
+  bool receiver_is_non_ptr = receiver->get_kind () != TyTy::TypeKind::POINTER;
+  if (autoderef_flag && receiver_is_non_ptr)
+    {
+      // it is unsafe to autoderef to raw pointers
+      return CoercionResult::get_error ();
+    }
+
   if (!coerceable_mutability (from_mutbl, to_mutbl))
     {
       Location lhs = mappings->lookup_location (receiver->get_ref ());
@@ -192,9 +227,9 @@ TypeCoercionRules::coerce_unsafe_ptr (TyTy::BaseType *receiver,
   TyTy::BaseType *result
     = unify_site_and (receiver->get_ref (), TyTy::TyWithLocation (expected),
                      TyTy::TyWithLocation (coerced_mutability),
-                     Location () /*unify_locus*/, false /*emit_errors*/,
-                     true /*commit_if_ok*/, true /*infer*/,
-                     true /*cleanup on error*/);
+                     locus /*unify_locus*/, false /*emit_errors*/,
+                     !try_flag /*commit_if_ok*/, true /*infer*/,
+                     try_flag /*cleanup on error*/);
   bool unsafe_ptr_coerceion_ok = result->get_kind () != TyTy::TypeKind::ERROR;
   if (unsafe_ptr_coerceion_ok)
     return CoercionResult{{}, result};
@@ -229,8 +264,12 @@ TypeCoercionRules::coerce_borrowed_pointer (TyTy::BaseType *receiver,
        // back to a final unity anyway
        rust_debug ("coerce_borrowed_pointer -- unify");
        TyTy::BaseType *result
-         = unify_site (receiver->get_ref (), TyTy::TyWithLocation (receiver),
-                       TyTy::TyWithLocation (expected), locus);
+         = unify_site_and (receiver->get_ref (),
+                           TyTy::TyWithLocation (receiver),
+                           TyTy::TyWithLocation (expected), locus,
+                           false /*emit_errors*/, true /*commit_if_ok*/,
+                           false /* FIXME infer do we want to allow this?? */,
+                           true /*cleanup_on_failure*/);
        return CoercionResult{{}, result};
       }
     }
index 0a55b8598d8cf9c24e50e8ad21907d1232f0ae8c..5a6dc64b3fccad848351fe0039e9a92a642a32f3 100644 (file)
@@ -69,7 +69,7 @@ public:
 
 protected:
   TypeCoercionRules (TyTy::BaseType *expected, Location locus, bool emit_errors,
-                    bool allow_autoderef);
+                    bool allow_autoderef, bool try_flag);
 
   bool select (TyTy::BaseType &autoderefed) override;
 
@@ -87,6 +87,7 @@ private:
   // mutable fields
   CoercionResult try_result;
   bool emit_errors;
+  bool try_flag;
 };
 
 } // namespace Resolver
index 4a291e13e5b73f7ecc78440f56abfa8b1689770f..6e5ab7d7a90c7719211d12353e33b08a258106be 100644 (file)
@@ -19,6 +19,8 @@
 #include "rust-hir-dot-operator.h"
 #include "rust-hir-path-probe.h"
 #include "rust-hir-trait-resolve.h"
+#include "rust-hir-type-check-item.h"
+#include "rust-coercion.h"
 
 namespace Rust {
 namespace Resolver {
@@ -55,6 +57,10 @@ MethodResolver::select (TyTy::BaseType &receiver)
     TyTy::FnType *ty;
   };
 
+  const TyTy::BaseType *raw = receiver.destructure ();
+  bool receiver_is_raw_ptr = raw->get_kind () == TyTy::TypeKind::POINTER;
+  bool receiver_is_ref = raw->get_kind () == TyTy::TypeKind::REF;
+
   // assemble inherent impl items
   std::vector<impl_item_candidate> inherent_impl_fns;
   mappings->iterate_impl_items (
@@ -86,6 +92,38 @@ MethodResolver::select (TyTy::BaseType &receiver)
 
       rust_assert (ty->get_kind () == TyTy::TypeKind::FNDEF);
       TyTy::FnType *fnty = static_cast<TyTy::FnType *> (ty);
+      const TyTy::BaseType *impl_self
+       = TypeCheckItem::ResolveImplBlockSelf (*impl);
+
+      // see:
+      // https://gcc-rust.zulipchat.com/#narrow/stream/266897-general/topic/Method.20Resolution/near/338646280
+      // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/method/probe.rs#L650-L660
+      bool impl_self_is_ptr = impl_self->get_kind () == TyTy::TypeKind::POINTER;
+      bool impl_self_is_ref = impl_self->get_kind () == TyTy::TypeKind::REF;
+      if (receiver_is_raw_ptr && impl_self_is_ptr)
+       {
+         const TyTy::PointerType &sptr
+           = *static_cast<const TyTy::PointerType *> (impl_self);
+         const TyTy::PointerType &ptr
+           = *static_cast<const TyTy::PointerType *> (raw);
+
+         // we could do this via lang-item assemblies if we refactor this
+         bool mut_match = sptr.mutability () == ptr.mutability ();
+         if (!mut_match)
+           return true;
+       }
+      else if (receiver_is_ref && impl_self_is_ref)
+       {
+         const TyTy::ReferenceType &sptr
+           = *static_cast<const TyTy::ReferenceType *> (impl_self);
+         const TyTy::ReferenceType &ptr
+           = *static_cast<const TyTy::ReferenceType *> (raw);
+
+         // we could do this via lang-item assemblies if we refactor this
+         bool mut_match = sptr.mutability () == ptr.mutability ();
+         if (!mut_match)
+           return true;
+       }
 
       inherent_impl_fns.push_back ({func, impl, fnty});
 
@@ -133,6 +171,39 @@ MethodResolver::select (TyTy::BaseType &receiver)
 
        rust_assert (ty->get_kind () == TyTy::TypeKind::FNDEF);
        TyTy::FnType *fnty = static_cast<TyTy::FnType *> (ty);
+       const TyTy::BaseType *impl_self
+         = TypeCheckItem::ResolveImplBlockSelf (*impl);
+
+       // see:
+       // https://gcc-rust.zulipchat.com/#narrow/stream/266897-general/topic/Method.20Resolution/near/338646280
+       // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/method/probe.rs#L650-L660
+       bool impl_self_is_ptr
+         = impl_self->get_kind () == TyTy::TypeKind::POINTER;
+       bool impl_self_is_ref = impl_self->get_kind () == TyTy::TypeKind::REF;
+       if (receiver_is_raw_ptr && impl_self_is_ptr)
+         {
+           const TyTy::PointerType &sptr
+             = *static_cast<const TyTy::PointerType *> (impl_self);
+           const TyTy::PointerType &ptr
+             = *static_cast<const TyTy::PointerType *> (raw);
+
+           // we could do this via lang-item assemblies if we refactor this
+           bool mut_match = sptr.mutability () == ptr.mutability ();
+           if (!mut_match)
+             continue;
+         }
+       else if (receiver_is_ref && impl_self_is_ref)
+         {
+           const TyTy::ReferenceType &sptr
+             = *static_cast<const TyTy::ReferenceType *> (impl_self);
+           const TyTy::ReferenceType &ptr
+             = *static_cast<const TyTy::ReferenceType *> (raw);
+
+           // we could do this via lang-item assemblies if we refactor this
+           bool mut_match = sptr.mutability () == ptr.mutability ();
+           if (!mut_match)
+             continue;
+         }
 
        inherent_impl_fns.push_back ({func, impl, fnty});
        return true;
@@ -170,18 +241,51 @@ MethodResolver::select (TyTy::BaseType &receiver)
     TyTy::FnType *fntype;
   };
 
+  // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/method/probe.rs#L580-L694
+
   rust_debug ("inherent_impl_fns found {%lu}, trait_fns found {%lu}, "
              "predicate_items found {%lu}",
              (unsigned long) inherent_impl_fns.size (),
              (unsigned long) trait_fns.size (),
              (unsigned long) predicate_items.size ());
 
-  // see the follow for the proper fix to get rid of this we need to assemble
-  // candidates based on a match expression gathering the relevant impl blocks
-  // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/method/probe.rs#L580-L694
-  TyTy::set_cmp_autoderef_mode ();
-
   bool found_possible_candidate = false;
+  for (const auto &predicate : predicate_items)
+    {
+      const TyTy::FnType *fn = predicate.fntype;
+      rust_assert (fn->is_method ());
+
+      TyTy::BaseType *fn_self = fn->get_self_type ();
+      rust_debug ("dot-operator predicate fn_self={%s} can_eq receiver={%s}",
+                 fn_self->debug_str ().c_str (),
+                 receiver.debug_str ().c_str ());
+
+      auto res = TypeCoercionRules::TryCoerce (&receiver, fn_self, Location (),
+                                              false /*allow-autoderef*/);
+      bool ok = !res.is_error ();
+      if (ok)
+       {
+         std::vector<Adjustment> adjs = append_adjustments (res.adjustments);
+         const TraitReference *trait_ref
+           = predicate.lookup.get_parent ()->get ();
+         const TraitItemReference *trait_item
+           = predicate.lookup.get_raw_item ();
+
+         PathProbeCandidate::TraitItemCandidate c{trait_ref, trait_item,
+                                                  nullptr};
+         auto try_result = MethodCandidate{
+           PathProbeCandidate (PathProbeCandidate::CandidateType::TRAIT_FUNC,
+                               fn->clone (), trait_item->get_locus (), c),
+           adjs};
+         result.insert (std::move (try_result));
+         found_possible_candidate = true;
+       }
+    }
+  if (found_possible_candidate)
+    {
+      return true;
+    }
+
   for (auto &impl_item : inherent_impl_fns)
     {
       bool is_trait_impl_block = impl_item.impl_block->has_trait_ref ();
@@ -195,21 +299,25 @@ MethodResolver::select (TyTy::BaseType &receiver)
       rust_debug ("dot-operator impl_item fn_self={%s} can_eq receiver={%s}",
                  fn_self->debug_str ().c_str (),
                  receiver.debug_str ().c_str ());
-      if (fn_self->can_eq (&receiver, false))
+
+      auto res = TypeCoercionRules::TryCoerce (&receiver, fn_self, Location (),
+                                              false /*allow-autoderef*/);
+      bool ok = !res.is_error ();
+      if (ok)
        {
+         std::vector<Adjustment> adjs = append_adjustments (res.adjustments);
          PathProbeCandidate::ImplItemCandidate c{impl_item.item,
                                                  impl_item.impl_block};
          auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::IMPL_FUNC,
                                fn, impl_item.item->get_locus (), c),
-           adjustments};
+           adjs};
          result.insert (std::move (try_result));
          found_possible_candidate = true;
        }
     }
   if (found_possible_candidate)
     {
-      TyTy::reset_cmp_autoderef_mode ();
       return true;
     }
 
@@ -226,21 +334,25 @@ MethodResolver::select (TyTy::BaseType &receiver)
       rust_debug (
        "dot-operator trait_impl_item fn_self={%s} can_eq receiver={%s}",
        fn_self->debug_str ().c_str (), receiver.debug_str ().c_str ());
-      if (fn_self->can_eq (&receiver, false))
+
+      auto res = TypeCoercionRules::TryCoerce (&receiver, fn_self, Location (),
+                                              false /*allow-autoderef*/);
+      bool ok = !res.is_error ();
+      if (ok)
        {
+         std::vector<Adjustment> adjs = append_adjustments (res.adjustments);
          PathProbeCandidate::ImplItemCandidate c{impl_item.item,
                                                  impl_item.impl_block};
          auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::IMPL_FUNC,
                                fn, impl_item.item->get_locus (), c),
-           adjustments};
+           adjs};
          result.insert (std::move (try_result));
          found_possible_candidate = true;
        }
     }
   if (found_possible_candidate)
     {
-      TyTy::reset_cmp_autoderef_mode ();
       return true;
     }
 
@@ -253,53 +365,25 @@ MethodResolver::select (TyTy::BaseType &receiver)
       rust_debug ("dot-operator trait_item fn_self={%s} can_eq receiver={%s}",
                  fn_self->debug_str ().c_str (),
                  receiver.debug_str ().c_str ());
-      if (fn_self->can_eq (&receiver, false))
+
+      auto res = TypeCoercionRules::TryCoerce (&receiver, fn_self, Location (),
+                                              false /*allow-autoderef*/);
+      bool ok = !res.is_error ();
+      if (ok)
        {
+         std::vector<Adjustment> adjs = append_adjustments (res.adjustments);
          PathProbeCandidate::TraitItemCandidate c{trait_item.reference,
                                                   trait_item.item_ref,
                                                   nullptr};
          auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::TRAIT_FUNC,
                                fn, trait_item.item->get_locus (), c),
-           adjustments};
-         result.insert (std::move (try_result));
-         found_possible_candidate = true;
-       }
-    }
-  if (found_possible_candidate)
-    {
-      TyTy::reset_cmp_autoderef_mode ();
-      return true;
-    }
-
-  for (const auto &predicate : predicate_items)
-    {
-      const TyTy::FnType *fn = predicate.fntype;
-      rust_assert (fn->is_method ());
-
-      TyTy::BaseType *fn_self = fn->get_self_type ();
-      rust_debug ("dot-operator predicate fn_self={%s} can_eq receiver={%s}",
-                 fn_self->debug_str ().c_str (),
-                 receiver.debug_str ().c_str ());
-      if (fn_self->can_eq (&receiver, false))
-       {
-         const TraitReference *trait_ref
-           = predicate.lookup.get_parent ()->get ();
-         const TraitItemReference *trait_item
-           = predicate.lookup.get_raw_item ();
-
-         PathProbeCandidate::TraitItemCandidate c{trait_ref, trait_item,
-                                                  nullptr};
-         auto try_result = MethodCandidate{
-           PathProbeCandidate (PathProbeCandidate::CandidateType::TRAIT_FUNC,
-                               fn->clone (), trait_item->get_locus (), c),
-           adjustments};
+           adjs};
          result.insert (std::move (try_result));
          found_possible_candidate = true;
        }
     }
 
-  TyTy::reset_cmp_autoderef_mode ();
   return found_possible_candidate;
 }
 
@@ -328,5 +412,19 @@ MethodResolver::get_predicate_items (
   return predicate_items;
 }
 
+std::vector<Adjustment>
+MethodResolver::append_adjustments (const std::vector<Adjustment> &adjs) const
+{
+  std::vector<Adjustment> combined;
+  combined.reserve (adjustments.size () + adjs.size ());
+
+  for (const auto &a : adjustments)
+    combined.push_back (a);
+  for (const auto &a : adjs)
+    combined.push_back (a);
+
+  return combined;
+}
+
 } // namespace Resolver
 } // namespace Rust
index 75927ff5ae23ccb10a9599572114afb24e3d7356..db04ad8a56fb5af74e018f594ade6462f77be619 100644 (file)
@@ -69,6 +69,10 @@ protected:
 
   bool select (TyTy::BaseType &receiver) override;
 
+private:
+  std::vector<Adjustment>
+  append_adjustments (const std::vector<Adjustment> &adjustments) const;
+
 private:
   // search
   const HIR::PathIdentSegment &segment_name;
index d4eea7ae954e708d39dc0a045529cf2d594feb0c..a4098682668a9c2cb2c85bd70a04dbefa78a47e7 100644 (file)
@@ -1027,6 +1027,8 @@ TypeCheckExpr::visit (HIR::MethodCallExpr &expr)
 
   context->insert_receiver (expr.get_mappings ().get_hirid (), receiver_tyty);
 
+  rust_debug_loc (expr.get_locus (), "attempting to resolve method for %s",
+                 receiver_tyty->debug_str ().c_str ());
   auto candidates
     = MethodResolver::Probe (receiver_tyty,
                             expr.get_method_name ().get_segment ());
@@ -1053,13 +1055,17 @@ TypeCheckExpr::visit (HIR::MethodCallExpr &expr)
 
   auto candidate = *candidates.begin ();
   rust_debug_loc (expr.get_method_name ().get_locus (),
-                 "resolved method to: {%u} {%s}",
+                 "resolved method to: {%u} {%s} with [%zu] adjustments",
                  candidate.candidate.ty->get_ref (),
-                 candidate.candidate.ty->debug_str ().c_str ());
+                 candidate.candidate.ty->debug_str ().c_str (),
+                 candidate.adjustments.size ());
 
   // Get the adjusted self
   Adjuster adj (receiver_tyty);
   TyTy::BaseType *adjusted_self = adj.adjust_type (candidate.adjustments);
+  rust_debug ("receiver: %s adjusted self %s",
+             receiver_tyty->debug_str ().c_str (),
+             adjusted_self->debug_str ().c_str ());
 
   // store the adjustments for code-generation to know what to do which must be
   // stored onto the receiver to so as we don't trigger duplicate deref mappings
@@ -1331,6 +1337,7 @@ TypeCheckExpr::visit (HIR::DereferenceExpr &expr)
   TyTy::BaseType *resolved_base
     = TypeCheckExpr::Resolve (expr.get_expr ().get ());
 
+  rust_debug_loc (expr.get_locus (), "attempting deref operator overload");
   auto lang_item_type = Analysis::RustLangItem::ItemType::DEREF;
   bool operator_overloaded
     = resolve_operator_overload (lang_item_type, expr, resolved_base, nullptr);
diff --git a/gcc/testsuite/rust/compile/issue-1901.rs b/gcc/testsuite/rust/compile/issue-1901.rs
new file mode 100644 (file)
index 0000000..b2a7080
--- /dev/null
@@ -0,0 +1,33 @@
+mod intrinsics {
+    extern "rust-intrinsic" {
+        pub fn offset<T>(ptr: *const T, count: isize) -> *const T;
+    }
+}
+
+mod ptr {
+    #[lang = "const_ptr"]
+    impl<T> *const T {
+        pub unsafe fn offset(self, count: isize) -> *const T {
+            intrinsics::offset(self, count)
+        }
+    }
+
+    #[lang = "mut_ptr"]
+    impl<T> *mut T {
+        pub unsafe fn offset(self, count: isize) -> *mut T {
+            intrinsics::offset(self, count) as *mut T
+        }
+    }
+}
+
+pub fn test_const(x: *const u8) {
+    unsafe {
+        x.offset(1);
+    }
+}
+
+pub fn test_mut(x: *mut u8) {
+    unsafe {
+        x.offset(1);
+    }
+}