public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Thomas Schwinge <tschwinge@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] Handle generic substitution on path expressions Date: Wed, 8 Jun 2022 12:04:41 +0000 (GMT) [thread overview] Message-ID: <20220608120441.E92413AA991A@sourceware.org> (raw) https://gcc.gnu.org/g:257bf55827474e500d83d96b7a8f8ebacbd11e7d commit 257bf55827474e500d83d96b7a8f8ebacbd11e7d Author: Philip Herron <philip.herron@embecosm.com> Date: Tue Feb 8 11:12:32 2022 +0000 Handle generic substitution on path expressions In this bug the path expression failed to take Foo::<i32> and apply this bound generic argument into the impl block. The fn type for this function test is: fn <T,Y>test(a:T, b:Y); But the impl block has a Self of Foo<T> so we need to inherit the T argument from the previous Foo::<i32> which was missing. Fixes #893 Diff: --- gcc/rust/typecheck/rust-hir-type-check-path.cc | 85 +++++++++++++++++------ gcc/rust/typecheck/rust-substitution-mapper.cc | 18 +++++ gcc/rust/typecheck/rust-substitution-mapper.h | 3 + gcc/rust/typecheck/rust-tyty.cc | 39 +++++++++++ gcc/rust/typecheck/rust-tyty.h | 51 ++++++++++++++ gcc/testsuite/rust/compile/torture/issue-893-2.rs | 35 ++++++++++ gcc/testsuite/rust/compile/torture/issue-893.rs | 11 +++ 7 files changed, 219 insertions(+), 23 deletions(-) diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc index f00cd16ca73..cd6d67a56ff 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-path.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc @@ -133,11 +133,6 @@ TypeCheckExpr::visit (HIR::PathInExpression &expr) size_t offset = -1; TyTy::BaseType *tyseg = resolve_root_path (expr, &offset, &resolved_node_id); - - if (tyseg == nullptr) - { - rust_debug_loc (expr.get_locus (), "failed to resolve root_seg"); - } rust_assert (tyseg != nullptr); if (tyseg->get_kind () == TyTy::TypeKind::ERROR) @@ -318,12 +313,12 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, { NodeId resolved_node_id = root_resolved_node_id; TyTy::BaseType *prev_segment = tyseg; + bool reciever_is_generic = prev_segment->get_kind () == TyTy::TypeKind::PARAM; + for (size_t i = offset; i < segments.size (); i++) { HIR::PathExprSegment &seg = segments.at (i); - bool reciever_is_generic - = prev_segment->get_kind () == TyTy::TypeKind::PARAM; bool probe_bounds = true; bool probe_impls = !reciever_is_generic; bool ignore_mandatory_trait_items = !reciever_is_generic; @@ -359,6 +354,7 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, prev_segment = tyseg; tyseg = candidate.ty; + HIR::ImplBlock *associated_impl_block = nullptr; if (candidate.is_enum_candidate ()) { const TyTy::VariantDef *variant = candidate.item.enum_field.variant; @@ -380,6 +376,8 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, { resolved_node_id = candidate.item.impl.impl_item->get_impl_mappings ().get_nodeid (); + + associated_impl_block = candidate.item.impl.parent; } else { @@ -403,6 +401,52 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, // we need a new ty_ref_id for this trait item tyseg = tyseg->clone (); tyseg->set_ty_ref (mappings->get_next_hir_id ()); + + // get the associated impl block + associated_impl_block = impl; + } + } + + if (associated_impl_block != nullptr) + { + // get the type of the parent Self + HirId impl_ty_id + = associated_impl_block->get_type ()->get_mappings ().get_hirid (); + TyTy::BaseType *impl_block_ty = nullptr; + bool ok = context->lookup_type (impl_ty_id, &impl_block_ty); + rust_assert (ok); + + if (prev_segment->needs_generic_substitutions ()) + { + if (!impl_block_ty->needs_generic_substitutions ()) + { + prev_segment = impl_block_ty; + } + else + { + HIR::PathExprSegment &pseg = segments.at (i - 1); + Location locus = pseg.get_locus (); + prev_segment = SubstMapper::InferSubst (prev_segment, locus); + } + } + } + + if (tyseg->needs_generic_substitutions ()) + { + if (!prev_segment->needs_generic_substitutions ()) + { + auto used_args_in_prev_segment + = GetUsedSubstArgs::From (prev_segment); + + if (!used_args_in_prev_segment.is_error ()) + { + if (SubstMapperInternal::mappings_are_bound ( + tyseg, used_args_in_prev_segment)) + { + tyseg = SubstMapperInternal::Resolve ( + tyseg, used_args_in_prev_segment); + } + } } } @@ -420,30 +464,25 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, if (tyseg->get_kind () == TyTy::TypeKind::ERROR) return; } - } - - context->insert_receiver (expr_mappings.get_hirid (), prev_segment); - if (tyseg->needs_generic_substitutions ()) - { - Location locus = segments.back ().get_locus (); - if (!prev_segment->needs_generic_substitutions ()) - { - auto used_args_in_prev_segment - = GetUsedSubstArgs::From (prev_segment); - if (!used_args_in_prev_segment.is_error ()) - tyseg - = SubstMapperInternal::Resolve (tyseg, used_args_in_prev_segment); - } - else + else if (tyseg->needs_generic_substitutions () && !reciever_is_generic) { + Location locus = seg.get_locus (); tyseg = SubstMapper::InferSubst (tyseg, locus); + if (tyseg->get_kind () == TyTy::TypeKind::ERROR) + return; } + } + rust_assert (resolved_node_id != UNKNOWN_NODEID); + if (tyseg->needs_generic_substitutions () && !reciever_is_generic) + { + Location locus = segments.back ().get_locus (); + tyseg = SubstMapper::InferSubst (tyseg, locus); if (tyseg->get_kind () == TyTy::TypeKind::ERROR) return; } - rust_assert (resolved_node_id != UNKNOWN_NODEID); + context->insert_receiver (expr_mappings.get_hirid (), prev_segment); // lookup if the name resolver was able to canonically resolve this or not NodeId path_resolved_id = UNKNOWN_NODEID; diff --git a/gcc/rust/typecheck/rust-substitution-mapper.cc b/gcc/rust/typecheck/rust-substitution-mapper.cc index 430eeeb66ca..f96b9b3ee7a 100644 --- a/gcc/rust/typecheck/rust-substitution-mapper.cc +++ b/gcc/rust/typecheck/rust-substitution-mapper.cc @@ -43,5 +43,23 @@ SubstMapperInternal::Resolve (TyTy::BaseType *base, return mapper.resolved; } +bool +SubstMapperInternal::mappings_are_bound ( + TyTy::BaseType *tyseg, TyTy::SubstitutionArgumentMappings &mappings) +{ + if (tyseg->get_kind () == TyTy::TypeKind::ADT) + { + TyTy::ADTType *adt = static_cast<TyTy::ADTType *> (tyseg); + return adt->are_mappings_bound (mappings); + } + else if (tyseg->get_kind () == TyTy::TypeKind::FNDEF) + { + TyTy::FnType *fn = static_cast<TyTy::FnType *> (tyseg); + return fn->are_mappings_bound (mappings); + } + + return false; +} + } // namespace Resolver } // namespace Rust diff --git a/gcc/rust/typecheck/rust-substitution-mapper.h b/gcc/rust/typecheck/rust-substitution-mapper.h index 310524552b1..4ea4762b4b3 100644 --- a/gcc/rust/typecheck/rust-substitution-mapper.h +++ b/gcc/rust/typecheck/rust-substitution-mapper.h @@ -156,6 +156,9 @@ public: static TyTy::BaseType *Resolve (TyTy::BaseType *base, TyTy::SubstitutionArgumentMappings &mappings); + static bool mappings_are_bound (TyTy::BaseType *ty, + TyTy::SubstitutionArgumentMappings &mappings); + void visit (TyTy::FnType &type) override { TyTy::SubstitutionArgumentMappings adjusted diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index 6a659239639..2dae0134f72 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -562,6 +562,45 @@ SubstitutionRef::adjust_mappings_for_this ( mappings.get_locus ()); } +bool +SubstitutionRef::are_mappings_bound (SubstitutionArgumentMappings &mappings) +{ + std::vector<SubstitutionArg> resolved_mappings; + for (size_t i = 0; i < substitutions.size (); i++) + { + auto &subst = substitutions.at (i); + + SubstitutionArg arg = SubstitutionArg::error (); + if (mappings.size () == substitutions.size ()) + { + mappings.get_argument_at (i, &arg); + } + else + { + if (subst.needs_substitution ()) + { + // get from passed in mappings + mappings.get_argument_for_symbol (subst.get_param_ty (), &arg); + } + else + { + // we should already have this somewhere + used_arguments.get_argument_for_symbol (subst.get_param_ty (), + &arg); + } + } + + bool ok = !arg.is_error (); + if (ok) + { + SubstitutionArg adjusted (&subst, arg.get_tyty ()); + resolved_mappings.push_back (std::move (adjusted)); + } + } + + return !resolved_mappings.empty (); +} + // this function assumes that the mappings being passed are for the same type as // this new substitution reference so ordering matters here SubstitutionArgumentMappings diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index 6c9daf73cae..b6f952ecc53 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -980,6 +980,57 @@ public: SubstitutionArgumentMappings adjust_mappings_for_this (SubstitutionArgumentMappings &mappings); + // Are the mappings here actually bound to this type. For example imagine the + // case: + // + // struct Foo<T>(T); + // impl<T> Foo<T> { + // fn test(self) { ... } + // } + // + // In this case we have a generic ADT of Foo and an impl block of a generic T + // on Foo for the Self type. When we it comes to path resolution we can have: + // + // Foo::<i32>::test() + // + // This means the first segment of Foo::<i32> returns the ADT Foo<i32> not the + // Self ADT bound to the T from the impl block. This means when it comes to + // the next segment of test which resolves to the function we need to check + // wether the arguments in the struct definition of foo can be bound here + // before substituting the previous segments type here. This functions acts as + // a guard for the solve_mappings_from_receiver_for_self to handle the case + // where arguments are not bound. This is important for this next case: + // + // struct Baz<A, B>(A, B); + // impl Baz<i32, f32> { + // fn test<X>(a: X) -> X { + // a + // } + // } + // + // In this case Baz has been already substituted for the impl's Self to become + // ADT<i32, f32> so that the function test only has 1 generic argument of X. + // The path for this will be: + // + // Baz::test::<_>(123) + // + // So the first segment here will be Baz<_, _> to try and infer the arguments + // which will be taken from the impl's Self type in this case since it is + // already substituted and like the previous case the check to see if we need + // to inherit the previous segments generic arguments takes place but the + // generic arguments are not bound to this type as they have already been + // substituted. + // + // Its important to remember from the first example the FnType actually looks + // like: + // + // fn <T>test(self :Foo<T>(T)) + // + // As the generic parameters are "bound" to each of the items in the impl + // block. So this check is about wether the arguments we have here can + // actually be bound to this type. + bool are_mappings_bound (SubstitutionArgumentMappings &mappings); + // struct Foo<A, B>(A, B); // // impl<T> Foo<T, f32>; diff --git a/gcc/testsuite/rust/compile/torture/issue-893-2.rs b/gcc/testsuite/rust/compile/torture/issue-893-2.rs new file mode 100644 index 00000000000..88a865d66dc --- /dev/null +++ b/gcc/testsuite/rust/compile/torture/issue-893-2.rs @@ -0,0 +1,35 @@ +// { dg-additional-options "-w" } +struct Foo<T>(T); +impl<T> Foo<T> { + fn new<Y>(a: T, b: Y) -> Self { + Self(a) + } +} + +struct Bar<T>(T); +impl Bar<i32> { + fn baz(self) {} + + fn test() -> i32 { + 123 + } +} + +struct Baz<A, B>(A, B); +impl Baz<i32, f32> { + fn test<X>(a: X) -> X { + a + } +} + +pub fn main() { + let a = Foo::<i32>::new::<f32>(123, 456f32); + let b = Foo::new::<f32>(123, 456f32); + + let c = Bar::<i32>(123); + let d = Bar::baz(c); + + let e = Bar::test(); + + let f = Baz::test::<bool>(true); +} diff --git a/gcc/testsuite/rust/compile/torture/issue-893.rs b/gcc/testsuite/rust/compile/torture/issue-893.rs new file mode 100644 index 00000000000..d8245f3e0d8 --- /dev/null +++ b/gcc/testsuite/rust/compile/torture/issue-893.rs @@ -0,0 +1,11 @@ +// { dg-additional-options "-w" } +struct Foo<T>(T); +impl<T> Foo<T> { + fn new<Y>(a: T, b: Y) -> Self { + Self(a) + } +} + +pub fn test() { + let a = Foo::<i32>::new::<f32>(123, 456f32); +}
reply other threads:[~2022-06-08 12:04 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220608120441.E92413AA991A@sourceware.org \ --to=tschwinge@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).