diff options
author | Philip Herron <philip.herron@embecosm.com> | 2022-02-08 11:12:32 +0000 |
---|---|---|
committer | Philip Herron <philip.herron@embecosm.com> | 2022-02-08 15:19:29 +0000 |
commit | 257bf55827474e500d83d96b7a8f8ebacbd11e7d (patch) | |
tree | badfdb2aaa7d640f8a5f43ad86d61e8fa6d878e1 /gcc | |
parent | 5619eea6f62bffe58eaaeb508f45a855fc3ce1f4 (diff) | |
download | gcc-257bf55827474e500d83d96b7a8f8ebacbd11e7d.zip gcc-257bf55827474e500d83d96b7a8f8ebacbd11e7d.tar.gz gcc-257bf55827474e500d83d96b7a8f8ebacbd11e7d.tar.bz2 |
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
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/rust/typecheck/rust-hir-type-check-path.cc | 85 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-substitution-mapper.cc | 18 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-substitution-mapper.h | 3 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-tyty.cc | 39 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-tyty.h | 51 | ||||
-rw-r--r-- | gcc/testsuite/rust/compile/torture/issue-893-2.rs | 35 | ||||
-rw-r--r-- | 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 f00cd16..cd6d67a 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 430eeeb..f96b9b3 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 3105245..4ea4762 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 6a65923..2dae013 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 6c9daf7..b6f952e 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 0000000..88a865d --- /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 0000000..d8245f3 --- /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); +} |