]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
gccrs: Method resolution must support multiple candidates
authorPhilip Herron <philip.herron@embecosm.com>
Thu, 13 Oct 2022 09:10:37 +0000 (10:10 +0100)
committerArthur Cohen <arthur.cohen@embecosm.com>
Tue, 21 Feb 2023 11:36:33 +0000 (12:36 +0100)
This patch fixes bad method resolution in our operator_overload_9 case.
When we have a &mut reference to something and we deref we must resolve to
the mutable reference impl block. The interface we are using to resolve
methods is the can_eq interface which allows for permissive mutability
which means allowing for mutable reference being unified with an immutable
one. This meant we actual match against both the immutable and mutable
version leading to multiple candidate error.

The fix here adds a method resolution flag to the can_eq interface so that
we enforce mutability equality. The other hack is that we do not allow
can_eq of ParamTypes to generic Slices. I think there is some subtle thing
going on for that case. The Rustc method resolver actually filters the
impl blocks for reference types based looking up the relevant lang items
we need to do this as well but is a much larger refactor to our method
resolver which should be done seperately.

Fixes #1588

gcc/rust/ChangeLog:

* typecheck/rust-autoderef.cc: Add support for multiple resolution candidates.
* typecheck/rust-hir-dot-operator.cc (MethodResolver::MethodResolver): Edit
`try_result` field and change constructor.
(MethodResolver::Probe): Return set of candidates instead of singular candidate.
(MethodResolver::select): Add better implementation to account for multiple
candidates.
* typecheck/rust-hir-dot-operator.h (struct MethodCandidate): Overload comparison
operator in order to store them in `std::set`.
* typecheck/rust-hir-inherent-impl-overlap.h: Do not fail assertion on missing type.
* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): Adapt code to use
multiple candidates.
* typecheck/rust-tyty.cc (set_cmp_autoderef_mode): Add code to handle automatic
derefs properly.
(reset_cmp_autoderef_mode): Add helper function to reset said mode.
* typecheck/rust-tyty.h (set_cmp_autoderef_mode): Declare function.
(reset_cmp_autoderef_mode): Likewise.
* typecheck/rust-tyty-cmp.h: Add handling of `autoderef_cmp_flag`

gcc/testsuite/ChangeLog:

* rust/compile/generics7.rs: Fix test with missing assertion.
* rust/execute/torture/operator_overload_9.rs: Fix test assertion.

gcc/rust/typecheck/rust-autoderef.cc
gcc/rust/typecheck/rust-hir-dot-operator.cc
gcc/rust/typecheck/rust-hir-dot-operator.h
gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h
gcc/rust/typecheck/rust-hir-type-check-expr.cc
gcc/rust/typecheck/rust-tyty-cmp.h
gcc/rust/typecheck/rust-tyty.cc
gcc/rust/typecheck/rust-tyty.h
gcc/testsuite/rust/compile/generics7.rs
gcc/testsuite/rust/execute/torture/operator_overload_9.rs

index ca43f847e98ee9658ce4f32fcb0249cbc2beb7af..fe05aeec3884fbaa9cb1c1a5140ea9736d767965 100644 (file)
@@ -139,15 +139,23 @@ resolve_operator_overload_fn (
     return false;
 
   auto segment = HIR::PathIdentSegment (associated_item_name);
-  auto candidate
+  auto candidates
     = MethodResolver::Probe (ty, HIR::PathIdentSegment (associated_item_name),
                             true);
 
-  bool have_implementation_for_lang_item = !candidate.is_error ();
+  bool have_implementation_for_lang_item = !candidates.empty ();
   if (!have_implementation_for_lang_item)
     return false;
 
+  // multiple candidates?
+  if (candidates.size () > 1)
+    {
+      // error out? probably not for this case
+      return false;
+    }
+
   // Get the adjusted self
+  auto candidate = *candidates.begin ();
   Adjuster adj (ty);
   TyTy::BaseType *adjusted_self = adj.adjust_type (candidate.adjustments);
 
index ed2df11da45f2195a89a8965eef01b4cf5d6e7fb..84fa8d4f08b6d0f8b4068b6fa9f6036c23a9d80e 100644 (file)
@@ -25,18 +25,17 @@ namespace Resolver {
 
 MethodResolver::MethodResolver (bool autoderef_flag,
                                const HIR::PathIdentSegment &segment_name)
-  : AutoderefCycle (autoderef_flag), segment_name (segment_name),
-    try_result (MethodCandidate::get_error ())
+  : AutoderefCycle (autoderef_flag), segment_name (segment_name), result ()
 {}
 
-MethodCandidate
+std::set<MethodCandidate>
 MethodResolver::Probe (const TyTy::BaseType *receiver,
                       const HIR::PathIdentSegment &segment_name,
                       bool autoderef_flag)
 {
   MethodResolver resolver (autoderef_flag, segment_name);
-  bool ok = resolver.cycle (receiver);
-  return ok ? resolver.try_result : MethodCandidate::get_error ();
+  resolver.cycle (receiver);
+  return resolver.result;
 }
 
 void
@@ -177,8 +176,18 @@ MethodResolver::select (const TyTy::BaseType &receiver)
              (unsigned long) trait_fns.size (),
              (unsigned long) predicate_items.size ());
 
-  for (auto impl_item : inherent_impl_fns)
+  // 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 (auto &impl_item : inherent_impl_fns)
     {
+      bool is_trait_impl_block = impl_item.impl_block->has_trait_ref ();
+      if (is_trait_impl_block)
+       continue;
+
       TyTy::FnType *fn = impl_item.ty;
       rust_assert (fn->is_method ());
 
@@ -190,13 +199,50 @@ MethodResolver::select (const TyTy::BaseType &receiver)
        {
          PathProbeCandidate::ImplItemCandidate c{impl_item.item,
                                                  impl_item.impl_block};
-         try_result = MethodCandidate{
+         auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::IMPL_FUNC,
                                fn, impl_item.item->get_locus (), c),
            adjustments};
-         return true;
+         result.insert (std::move (try_result));
+         found_possible_candidate = true;
        }
     }
+  if (found_possible_candidate)
+    {
+      TyTy::reset_cmp_autoderef_mode ();
+      return true;
+    }
+
+  for (auto &impl_item : inherent_impl_fns)
+    {
+      bool is_trait_impl_block = impl_item.impl_block->has_trait_ref ();
+      if (!is_trait_impl_block)
+       continue;
+
+      TyTy::FnType *fn = impl_item.ty;
+      rust_assert (fn->is_method ());
+
+      TyTy::BaseType *fn_self = fn->get_self_type ();
+      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))
+       {
+         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};
+         result.insert (std::move (try_result));
+         found_possible_candidate = true;
+       }
+    }
+  if (found_possible_candidate)
+    {
+      TyTy::reset_cmp_autoderef_mode ();
+      return true;
+    }
 
   for (auto trait_item : trait_fns)
     {
@@ -212,13 +258,19 @@ MethodResolver::select (const TyTy::BaseType &receiver)
          PathProbeCandidate::TraitItemCandidate c{trait_item.reference,
                                                   trait_item.item_ref,
                                                   nullptr};
-         try_result = MethodCandidate{
+         auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::TRAIT_FUNC,
                                fn, trait_item.item->get_locus (), c),
            adjustments};
-         return true;
+         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)
     {
@@ -238,15 +290,17 @@ MethodResolver::select (const TyTy::BaseType &receiver)
 
          PathProbeCandidate::TraitItemCandidate c{trait_ref, trait_item,
                                                   nullptr};
-         try_result = MethodCandidate{
+         auto try_result = MethodCandidate{
            PathProbeCandidate (PathProbeCandidate::CandidateType::TRAIT_FUNC,
                                fn->clone (), trait_item->get_locus (), c),
            adjustments};
-         return true;
+         result.insert (std::move (try_result));
+         found_possible_candidate = true;
        }
     }
 
-  return false;
+  TyTy::reset_cmp_autoderef_mode ();
+  return found_possible_candidate;
 }
 
 std::vector<MethodResolver::predicate_candidate>
index 8341173a061ff1ee09f1ea51266f6cc252006919..e14baf3f87d0e8c1650064c6cfa4b34bed49fa6b 100644 (file)
@@ -35,6 +35,13 @@ struct MethodCandidate
   }
 
   bool is_error () const { return candidate.is_error (); }
+
+  DefId get_defid () const { return candidate.get_defid (); }
+
+  bool operator< (const MethodCandidate &c) const
+  {
+    return get_defid () < c.get_defid ();
+  }
 };
 
 class MethodResolver : private TypeCheckBase, protected AutoderefCycle
@@ -46,9 +53,10 @@ public:
     TyTy::FnType *fntype;
   };
 
-  static MethodCandidate Probe (const TyTy::BaseType *receiver,
-                               const HIR::PathIdentSegment &segment_name,
-                               bool autoderef_flag = false);
+  static std::set<MethodCandidate>
+  Probe (const TyTy::BaseType *receiver,
+        const HIR::PathIdentSegment &segment_name,
+        bool autoderef_flag = false);
 
   static std::vector<predicate_candidate> get_predicate_items (
     const HIR::PathIdentSegment &segment_name, const TyTy::BaseType &receiver,
@@ -68,7 +76,7 @@ private:
   std::vector<MethodResolver::predicate_candidate> predicate_items;
 
   // mutable fields
-  MethodCandidate try_result;
+  std::set<MethodCandidate> result;
 };
 
 } // namespace Resolver
index 3733f555cfb9bb26c95f205d573be8e0c7f3d2f3..6e2fe1b2286eb4d77c7066df9cc4f0fd857672f2 100644 (file)
@@ -93,8 +93,9 @@ public:
 
     HirId impl_type_id = impl->get_type ()->get_mappings ().get_hirid ();
     TyTy::BaseType *impl_type = nullptr;
-    bool ok = context->lookup_type (impl_type_id, &impl_type);
-    rust_assert (ok);
+    bool ok = query_type (impl_type_id, &impl_type);
+    if (!ok)
+      return;
 
     std::string impl_item_name;
     ok = ImplItemToName::resolve (impl_item, impl_item_name);
index 2f22c82f06335278861b8837f046b536073f544b..4e377d52a0fb4a44f7bd300d4149aecf1dc3330b 100644 (file)
@@ -1015,10 +1015,10 @@ TypeCheckExpr::visit (HIR::MethodCallExpr &expr)
 
   context->insert_receiver (expr.get_mappings ().get_hirid (), receiver_tyty);
 
-  auto candidate
+  auto candidates
     = MethodResolver::Probe (receiver_tyty,
                             expr.get_method_name ().get_segment ());
-  if (candidate.is_error ())
+  if (candidates.empty ())
     {
       rust_error_at (
        expr.get_method_name ().get_locus (),
@@ -1027,6 +1027,19 @@ TypeCheckExpr::visit (HIR::MethodCallExpr &expr)
       return;
     }
 
+  if (candidates.size () > 1)
+    {
+      RichLocation r (expr.get_method_name ().get_locus ());
+      for (auto &c : candidates)
+       r.add_range (c.candidate.locus);
+
+      rust_error_at (
+       r, "multiple candidates found for method %<%s%>",
+       expr.get_method_name ().get_segment ().as_string ().c_str ());
+      return;
+    }
+
+  auto candidate = *candidates.begin ();
   rust_debug_loc (expr.get_method_name ().get_locus (),
                  "resolved method to: {%u} {%s}",
                  candidate.candidate.ty->get_ref (),
@@ -1422,14 +1435,28 @@ TypeCheckExpr::resolve_operator_overload (
     return false;
 
   auto segment = HIR::PathIdentSegment (associated_item_name);
-  auto candidate
+  auto candidates
     = MethodResolver::Probe (lhs, HIR::PathIdentSegment (associated_item_name));
 
-  bool have_implementation_for_lang_item = !candidate.is_error ();
+  bool have_implementation_for_lang_item = candidates.size () > 0;
   if (!have_implementation_for_lang_item)
     return false;
 
+  if (candidates.size () > 1)
+    {
+      // mutliple candidates
+      RichLocation r (expr.get_locus ());
+      for (auto &c : candidates)
+       r.add_range (c.candidate.locus);
+
+      rust_error_at (
+       r, "multiple candidates found for possible operator overload");
+
+      return false;
+    }
+
   // Get the adjusted self
+  auto candidate = *candidates.begin ();
   Adjuster adj (lhs);
   TyTy::BaseType *adjusted_self = adj.adjust_type (candidate.adjustments);
 
index d47cbb4369a84fe647d99903030f7b9f5e951846..5dfd2d7184aaa5200aea6a7e55daf0fc22e3cb8b 100644 (file)
 namespace Rust {
 namespace TyTy {
 
+// we need to fix this properly by implementing the match for assembling
+// candidates
+extern bool autoderef_cmp_flag;
+
 class BaseCmp : public TyConstVisitor
 {
 public:
@@ -1244,6 +1248,9 @@ public:
     auto other_base_type = type.get_base ();
 
     bool mutability_ok = base->is_mutable () ? type.is_mutable () : true;
+    if (autoderef_cmp_flag)
+      mutability_ok = base->mutability () == type.mutability ();
+
     if (!mutability_ok)
       {
        BaseCmp::visit (type);
@@ -1289,9 +1296,10 @@ public:
     auto base_type = base->get_base ();
     auto other_base_type = type.get_base ();
 
-    // rust is permissive about mutablity here you can always go from mutable to
-    // immutable but not the otherway round
     bool mutability_ok = base->is_mutable () ? type.is_mutable () : true;
+    if (autoderef_cmp_flag)
+      mutability_ok = base->mutability () == type.mutability ();
+
     if (!mutability_ok)
       {
        BaseCmp::visit (type);
@@ -1370,7 +1378,7 @@ public:
 
   void visit (const ArrayType &) override { ok = true; }
 
-  void visit (const SliceType &) override { ok = true; }
+  void visit (const SliceType &) override { ok = !autoderef_cmp_flag; }
 
   void visit (const BoolType &) override { ok = true; }
 
index e2f79971337f34fb43db9c7d5213b2176bc24aa6..462c5bf91fd19f1b53039106af69260238389226 100644 (file)
 namespace Rust {
 namespace TyTy {
 
+bool autoderef_cmp_flag = false;
+
+void
+set_cmp_autoderef_mode ()
+{
+  autoderef_cmp_flag = true;
+}
+void
+reset_cmp_autoderef_mode ()
+{
+  autoderef_cmp_flag = false;
+}
+
 std::string
 TypeKindFormat::to_string (TypeKind kind)
 {
index a033fcad6c9dacc21cefa8db49252767b3b656fe..b17f01f67eab178a026570c24e4079ad54b9e430 100644 (file)
@@ -135,6 +135,11 @@ protected:
   std::vector<TypeBoundPredicate> specified_bounds;
 };
 
+extern void
+set_cmp_autoderef_mode ();
+extern void
+reset_cmp_autoderef_mode ();
+
 class TyVisitor;
 class TyConstVisitor;
 class BaseType : public TypeBoundsMappings
index 2a41632e693a6df235170416087065c88c93765e..2789c30ee38421bea4c81b9f3fc44bc6de4f63aa 100644 (file)
@@ -3,13 +3,13 @@ struct Foo<A> {
 }
 
 impl Foo<isize> {
-    fn bar(self) -> isize { // { dg-error "duplicate definitions with name bar" }
+    fn bar(self) -> isize {
         self.a
     }
 }
 
 impl Foo<char> {
-    fn bar(self) -> char { // { dg-error "duplicate definitions with name bar" }
+    fn bar(self) -> char {
         self.a
     }
 }
@@ -23,4 +23,6 @@ impl<T> Foo<T> {
 fn main() {
     let a = Foo { a: 123 };
     a.bar();
+    // { dg-error "multiple candidates found for method .bar." "" { target *-*-* } .-1 }
+    // { dg-error "failed to type resolve expression" "" { target *-*-* } .-2 }
 }
index fbb96a60d3630f27904454a31aaaea6ddf2bdc6f..fd972e28ab35fb08558f0a13b3ebebc347446a22 100644 (file)
@@ -1,4 +1,4 @@
-/* { dg-output "imm_deref\n123\n" } */
+/* { dg-output "mut_deref\n123\n" } */
 extern "C" {
     fn printf(s: *const i8, ...);
 }