From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id 5F6C83858C50; Fri, 14 Oct 2022 12:45:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5F6C83858C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1665751558; bh=KUqmAzGk8wGdnqZp8RMbevJoAuLXuzP7s1Tpws6HRa8=; h=From:To:Subject:Date:From; b=yIxZk5MIhC56EUrkpHHR/KJz5S34h0f/HZytnXxwkalLH7i9jFnUaH+5HCzCx95Gt 87M7ZNtEZQaIVJIkqx9COrQIfY9hLeknzP+m5it14o6BiFdNsjx6iVvjGEnXNhiSJo 88ZmIaSRS8X4sre5k2w8kVgl6uT3wX4W8OJJ0x+0= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Thomas Schwinge To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] Method resolution must support multiple candidates X-Act-Checkin: gcc X-Git-Author: Philip Herron X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: 3e7320510e7bb774295ecbd8089a9f51d6475959 X-Git-Newrev: 9bac2dbfe999b0dffea65b9a1b6ed0a257edf3c8 Message-Id: <20221014124558.5F6C83858C50@sourceware.org> Date: Fri, 14 Oct 2022 12:45:58 +0000 (GMT) List-Id: https://gcc.gnu.org/g:9bac2dbfe999b0dffea65b9a1b6ed0a257edf3c8 commit 9bac2dbfe999b0dffea65b9a1b6ed0a257edf3c8 Author: Philip Herron Date: Thu Oct 13 10:10:37 2022 +0100 Method resolution must support multiple candidates 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 Diff: --- gcc/rust/typecheck/rust-autoderef.cc | 12 +++- gcc/rust/typecheck/rust-hir-dot-operator.cc | 80 ++++++++++++++++++---- gcc/rust/typecheck/rust-hir-dot-operator.h | 16 +++-- .../typecheck/rust-hir-inherent-impl-overlap.h | 5 +- gcc/rust/typecheck/rust-hir-type-check-expr.cc | 35 ++++++++-- gcc/rust/typecheck/rust-tyty-cmp.h | 14 +++- gcc/rust/typecheck/rust-tyty.cc | 13 ++++ gcc/rust/typecheck/rust-tyty.h | 5 ++ gcc/testsuite/rust/compile/generics7.rs | 6 +- .../rust/execute/torture/operator_overload_9.rs | 2 +- 10 files changed, 157 insertions(+), 31 deletions(-) diff --git a/gcc/rust/typecheck/rust-autoderef.cc b/gcc/rust/typecheck/rust-autoderef.cc index 3d6bc0b169d..7b176beaaeb 100644 --- a/gcc/rust/typecheck/rust-autoderef.cc +++ b/gcc/rust/typecheck/rust-autoderef.cc @@ -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); diff --git a/gcc/rust/typecheck/rust-hir-dot-operator.cc b/gcc/rust/typecheck/rust-hir-dot-operator.cc index 1976a5f6a34..46547aa20e0 100644 --- a/gcc/rust/typecheck/rust-hir-dot-operator.cc +++ b/gcc/rust/typecheck/rust-hir-dot-operator.cc @@ -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 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 diff --git a/gcc/rust/typecheck/rust-hir-dot-operator.h b/gcc/rust/typecheck/rust-hir-dot-operator.h index 054608dc931..4cf7217ea0b 100644 --- a/gcc/rust/typecheck/rust-hir-dot-operator.h +++ b/gcc/rust/typecheck/rust-hir-dot-operator.h @@ -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 + Probe (const TyTy::BaseType *receiver, + const HIR::PathIdentSegment &segment_name, + bool autoderef_flag = false); static std::vector get_predicate_items ( const HIR::PathIdentSegment &segment_name, const TyTy::BaseType &receiver, @@ -68,7 +76,7 @@ private: std::vector predicate_items; // mutable fields - MethodCandidate try_result; + std::set result; }; } // namespace Resolver diff --git a/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h b/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h index 2890b54a00d..823431eab4d 100644 --- a/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h +++ b/gcc/rust/typecheck/rust-hir-inherent-impl-overlap.h @@ -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); diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.cc b/gcc/rust/typecheck/rust-hir-type-check-expr.cc index 9a5785f71b7..3411a2daccb 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.cc @@ -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); diff --git a/gcc/rust/typecheck/rust-tyty-cmp.h b/gcc/rust/typecheck/rust-tyty-cmp.h index 1e5dcb5a2e3..cc8a180bdb0 100644 --- a/gcc/rust/typecheck/rust-tyty-cmp.h +++ b/gcc/rust/typecheck/rust-tyty-cmp.h @@ -28,6 +28,10 @@ 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; } diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index b9a93186208..5127768fdad 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -32,6 +32,19 @@ 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) { diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index acbeb9c441b..998ed63ff27 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -135,6 +135,11 @@ protected: std::vector specified_bounds; }; +extern void +set_cmp_autoderef_mode (); +extern void +reset_cmp_autoderef_mode (); + class TyVisitor; class TyConstVisitor; class BaseType : public TypeBoundsMappings diff --git a/gcc/testsuite/rust/compile/generics7.rs b/gcc/testsuite/rust/compile/generics7.rs index 2a41632e693..2789c30ee38 100644 --- a/gcc/testsuite/rust/compile/generics7.rs +++ b/gcc/testsuite/rust/compile/generics7.rs @@ -3,13 +3,13 @@ struct Foo { } impl Foo { - fn bar(self) -> isize { // { dg-error "duplicate definitions with name bar" } + fn bar(self) -> isize { self.a } } impl Foo { - fn bar(self) -> char { // { dg-error "duplicate definitions with name bar" } + fn bar(self) -> char { self.a } } @@ -23,4 +23,6 @@ impl Foo { 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 } } diff --git a/gcc/testsuite/rust/execute/torture/operator_overload_9.rs b/gcc/testsuite/rust/execute/torture/operator_overload_9.rs index fbb96a60d36..fd972e28ab3 100644 --- a/gcc/testsuite/rust/execute/torture/operator_overload_9.rs +++ b/gcc/testsuite/rust/execute/torture/operator_overload_9.rs @@ -1,4 +1,4 @@ -/* { dg-output "imm_deref\n123\n" } */ +/* { dg-output "mut_deref\n123\n" } */ extern "C" { fn printf(s: *const i8, ...); }