From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id 211003858401; Tue, 28 Feb 2023 22:37:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 211003858401 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1677623860; bh=V2Rj1D0M/lryZERALI1BprmvrsFUO/wFZF4HvPleu08=; h=From:To:Subject:Date:From; b=UyLkIdbv9eckEfpVR2lxcYJtcdNI5AU13K8dIgSVp5FfOxmRd2cNf4SkDraqlBmGh 5jPC52B64UJfzCdEweXB2L6BzsDS2TLE7+Lb060Hl8E1SSSQkL3GFyTUQjKtCGBh55 ekiLkrrB/FiX58GK9FkJxm5FuYu3chwU79brNZfY= 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] gccrs: Fix method resolution to use TryCoerce X-Act-Checkin: gcc X-Git-Author: Philip Herron X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: a20426036a360d781dae2361ffd4c7dcfe4bb40a X-Git-Newrev: 43a92787e3ff964ade498be3a6a1b6adc1f41087 Message-Id: <20230228223740.211003858401@sourceware.org> Date: Tue, 28 Feb 2023 22:37:40 +0000 (GMT) List-Id: https://gcc.gnu.org/g:43a92787e3ff964ade498be3a6a1b6adc1f41087 commit 43a92787e3ff964ade498be3a6a1b6adc1f41087 Author: Philip Herron Date: Mon Feb 27 19:16:00 2023 +0000 gccrs: Fix method resolution to use TryCoerce 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 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. Diff: --- gcc/rust/typecheck/rust-coercion.cc | 57 ++++++-- gcc/rust/typecheck/rust-coercion.h | 3 +- gcc/rust/typecheck/rust-hir-dot-operator.cc | 190 +++++++++++++++++++------ gcc/rust/typecheck/rust-hir-dot-operator.h | 4 + gcc/rust/typecheck/rust-hir-type-check-expr.cc | 11 +- gcc/testsuite/rust/compile/issue-1901.rs | 33 +++++ 6 files changed, 240 insertions(+), 58 deletions(-) diff --git a/gcc/rust/typecheck/rust-coercion.cc b/gcc/rust/typecheck/rust-coercion.cc index c07ee733514..9831e77cdd0 100644 --- a/gcc/rust/typecheck/rust-coercion.cc +++ b/gcc/rust/typecheck/rust-coercion.cc @@ -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}; } } diff --git a/gcc/rust/typecheck/rust-coercion.h b/gcc/rust/typecheck/rust-coercion.h index 0a55b8598d8..5a6dc64b3fc 100644 --- a/gcc/rust/typecheck/rust-coercion.h +++ b/gcc/rust/typecheck/rust-coercion.h @@ -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 diff --git a/gcc/rust/typecheck/rust-hir-dot-operator.cc b/gcc/rust/typecheck/rust-hir-dot-operator.cc index 4a291e13e5b..6e5ab7d7a90 100644 --- a/gcc/rust/typecheck/rust-hir-dot-operator.cc +++ b/gcc/rust/typecheck/rust-hir-dot-operator.cc @@ -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 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 (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 (impl_self); + const TyTy::PointerType &ptr + = *static_cast (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 (impl_self); + const TyTy::ReferenceType &ptr + = *static_cast (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 (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 (impl_self); + const TyTy::PointerType &ptr + = *static_cast (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 (impl_self); + const TyTy::ReferenceType &ptr + = *static_cast (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 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 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 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 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 +MethodResolver::append_adjustments (const std::vector &adjs) const +{ + std::vector 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 diff --git a/gcc/rust/typecheck/rust-hir-dot-operator.h b/gcc/rust/typecheck/rust-hir-dot-operator.h index 75927ff5ae2..db04ad8a56f 100644 --- a/gcc/rust/typecheck/rust-hir-dot-operator.h +++ b/gcc/rust/typecheck/rust-hir-dot-operator.h @@ -69,6 +69,10 @@ protected: bool select (TyTy::BaseType &receiver) override; +private: + std::vector + append_adjustments (const std::vector &adjustments) const; + private: // search const HIR::PathIdentSegment &segment_name; diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.cc b/gcc/rust/typecheck/rust-hir-type-check-expr.cc index d4eea7ae954..a4098682668 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.cc @@ -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 index 00000000000..b2a70805e75 --- /dev/null +++ b/gcc/testsuite/rust/compile/issue-1901.rs @@ -0,0 +1,33 @@ +mod intrinsics { + extern "rust-intrinsic" { + pub fn offset(ptr: *const T, count: isize) -> *const T; + } +} + +mod ptr { + #[lang = "const_ptr"] + impl *const T { + pub unsafe fn offset(self, count: isize) -> *const T { + intrinsics::offset(self, count) + } + } + + #[lang = "mut_ptr"] + impl *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); + } +}