diff options
author | Philip Herron <philip.herron@embecosm.com> | 2022-04-26 20:12:58 +0100 |
---|---|---|
committer | Philip Herron <philip.herron@embecosm.com> | 2022-04-26 21:44:00 +0100 |
commit | ea38a59ee8329bb7b309400a7de57d53ad7bde31 (patch) | |
tree | 7efe52d94bf719b0a4db1d15d0f90ec65e6e5141 /gcc | |
parent | 40f2979b73b56e4ddc5ab5686d75164a8b664320 (diff) | |
download | gcc-ea38a59ee8329bb7b309400a7de57d53ad7bde31.zip gcc-ea38a59ee8329bb7b309400a7de57d53ad7bde31.tar.gz gcc-ea38a59ee8329bb7b309400a7de57d53ad7bde31.tar.bz2 |
Fix regression in fix for #1173
A regression was introduced in e476433c45624ec713395c85bcbd410806e90639
this was a tricky bug to fix. There are several fixes here but they all
rely on one another so in order to have a commit which does not introduce
more regressions it seems best to keep these fixes together.
In this patch we remove the check at the end of type resolution to look
for any error type nodes within the type context since this will only ever
produce false postive/duplicate errors. Since any real error will have been
emitted at the time of occurance.
We also fixed a bug in infering the types on a generic type so for example:
struct Foo<A,B>(A,B);
impl<T> Foo<T,T> { ... }
In this example the Self type on the impl block will be Foo<T,T> so in the
code for infering the arguments for the type we simply iterated each of the
generic parameter mappings and if any of them need substitution we generate
implict inference variables for them but in this case the ParamType is T
so generating one for each of these T's will introduce a stay inference
variable which we cannot infer so it will not be used. This patch keeps
a mapping inside SubstitutionRef::infer_substitions to ensure we don't
introduce any extra inference variables than required.
The final fix was around how we clone inference variables, there is a
comment explaining this but we cannot safely clone a inference variables
simply by creating a new object, inference variables are closely tied to
the type-conext so that when we infer the type to be a concrete type we
need to be able to update the reference in memory but simply cloning an
inference variable does not guarentee this to occur. This change ensures
that we create a new implict inference variable and setup the reference
and chain apropirately.
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/rust/hir/tree/rust-hir-path.h | 2 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-hir-type-check-expr.h | 8 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-hir-type-check-path.cc | 106 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-hir-type-check.cc | 9 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-tyty-rules.h | 5 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-tyty.cc | 31 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-tyty.h | 15 | ||||
-rw-r--r-- | gcc/testsuite/rust/compile/torture/issue-893-2.rs | 2 |
8 files changed, 76 insertions, 102 deletions
diff --git a/gcc/rust/hir/tree/rust-hir-path.h b/gcc/rust/hir/tree/rust-hir-path.h index 84096ce..aa3e29a 100644 --- a/gcc/rust/hir/tree/rust-hir-path.h +++ b/gcc/rust/hir/tree/rust-hir-path.h @@ -262,6 +262,8 @@ public: std::vector<PathExprSegment> &get_segments () { return segments; } + const std::vector<PathExprSegment> &get_segments () const { return segments; } + PathExprSegment &get_root_seg () { return segments.at (0); } PathExprSegment get_final_segment () const { return segments.back (); } diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.h b/gcc/rust/typecheck/rust-hir-type-check-expr.h index 630eb60..5633751 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.h +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.h @@ -221,13 +221,6 @@ public: infered = TyTy::TypeCheckCallExpr::go (function_tyty, expr, variant, context); - if (infered == nullptr) - { - rust_error_at (expr.get_locus (), "failed to lookup type to CallExpr"); - return; - } - - infered->set_ref (expr.get_mappings ().get_hirid ()); } void visit (HIR::MethodCallExpr &expr) override @@ -1076,7 +1069,6 @@ private: : TypeCheckBase (), infered (nullptr), inside_loop (inside_loop) {} - // Beware: currently returns Tyty::ErrorType or nullptr in case of error. TyTy::BaseType *resolve_root_path (HIR::PathInExpression &expr, size_t *offset, NodeId *root_resolved_node_id); diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc index 530f630..9960e76 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-path.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc @@ -138,27 +138,17 @@ void TypeCheckExpr::visit (HIR::PathInExpression &expr) { NodeId resolved_node_id = UNKNOWN_NODEID; - size_t offset = -1; TyTy::BaseType *tyseg = resolve_root_path (expr, &offset, &resolved_node_id); - rust_assert (tyseg != nullptr); - if (tyseg->get_kind () == TyTy::TypeKind::ERROR) return; - if (expr.get_num_segments () == 1) - { - Location locus = expr.get_segments ().back ().get_locus (); - - bool is_big_self - = expr.get_segments ().front ().get_segment ().as_string ().compare ( - "Self") - == 0; - if (!is_big_self && tyseg->needs_generic_substitutions ()) - { - tyseg = SubstMapper::InferSubst (tyseg, locus); - } + if (tyseg->needs_generic_substitutions ()) + tyseg = SubstMapper::InferSubst (tyseg, expr.get_locus ()); + bool fully_resolved = offset == expr.get_segments ().size (); + if (fully_resolved) + { infered = tyseg; return; } @@ -171,14 +161,11 @@ TyTy::BaseType * TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset, NodeId *root_resolved_node_id) { - TyTy::BaseType *root_tyty = nullptr; *offset = 0; for (size_t i = 0; i < expr.get_num_segments (); i++) { HIR::PathExprSegment &seg = expr.get_segments ().at (i); - bool have_more_segments = (expr.get_num_segments () - 1 != i); - bool is_root = *offset == 0 || root_tyty == nullptr; NodeId ast_node_id = seg.get_mappings ().get_nodeid (); // then lookup the reference_node_id @@ -205,13 +192,9 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset, // ref_node_id is the NodeId that the segments refers to. if (ref_node_id == UNKNOWN_NODEID) { - if (is_root) - { - rust_error_at (seg.get_locus (), - "failed to type resolve root segment"); - return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); - } - return root_tyty; + rust_error_at (seg.get_locus (), + "failed to type resolve root segment"); + return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); } // node back to HIR @@ -219,26 +202,20 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset, if (!mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (), ref_node_id, &ref)) { - if (is_root) - { - rust_error_at (seg.get_locus (), "456 reverse lookup failure"); - rust_debug_loc ( - seg.get_locus (), - "failure with [%s] mappings [%s] ref_node_id [%u]", - seg.as_string ().c_str (), - seg.get_mappings ().as_string ().c_str (), ref_node_id); + rust_error_at (seg.get_locus (), "456 reverse lookup failure"); + rust_debug_loc (seg.get_locus (), + "failure with [%s] mappings [%s] ref_node_id [%u]", + seg.as_string ().c_str (), + seg.get_mappings ().as_string ().c_str (), + ref_node_id); - return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); - } - - return root_tyty; + return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); } auto seg_is_module = (nullptr != mappings->lookup_module (expr.get_mappings ().get_crate_num (), ref)); - if (seg_is_module) { // A::B::C::this_is_a_module::D::E::F @@ -261,33 +238,8 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset, TyTy::BaseType *lookup = nullptr; if (!context->lookup_type (ref, &lookup)) { - if (is_root) - { - rust_error_at (seg.get_locus (), - "failed to resolve root segment"); - return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); - } - return root_tyty; - } - - // if we have a previous segment type - if (root_tyty != nullptr) - { - // if this next segment needs substitution we must apply the - // previous type arguments - // - // such as: GenericStruct::<_>::new(123, 456) - if (lookup->needs_generic_substitutions ()) - { - if (!root_tyty->needs_generic_substitutions ()) - { - auto used_args_in_prev_segment - = GetUsedSubstArgs::From (root_tyty); - lookup - = SubstMapperInternal::Resolve (lookup, - used_args_in_prev_segment); - } - } + rust_error_at (seg.get_locus (), "failed to resolve root segment"); + return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); } // turbo-fish segment path::<ty> @@ -300,16 +252,16 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset, lookup->as_string ().c_str ()); return new TyTy::ErrorType (lookup->get_ref ()); } - lookup = SubstMapper::Resolve (lookup, expr.get_locus (), + lookup = SubstMapper::Resolve (lookup, seg.get_locus (), &seg.get_generic_args ()); } *root_resolved_node_id = ref_node_id; *offset = *offset + 1; - root_tyty = lookup; + return lookup; } - return root_tyty; + return new TyTy::ErrorType (expr.get_mappings ().get_hirid ()); } void @@ -424,19 +376,11 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id, 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 (impl_block_ty->needs_generic_substitutions ()) + impl_block_ty + = SubstMapper::InferSubst (impl_block_ty, seg.get_locus ()); + + prev_segment = prev_segment->unify (impl_block_ty); } if (tyseg->needs_generic_substitutions ()) diff --git a/gcc/rust/typecheck/rust-hir-type-check.cc b/gcc/rust/typecheck/rust-hir-type-check.cc index b09ffc7..ee66638 100644 --- a/gcc/rust/typecheck/rust-hir-type-check.cc +++ b/gcc/rust/typecheck/rust-hir-type-check.cc @@ -54,18 +54,11 @@ TypeResolution::Resolve (HIR::Crate &crate) // default inference variables if possible context->iterate ([&] (HirId id, TyTy::BaseType *ty) mutable -> bool { - if (ty->get_kind () == TyTy::TypeKind::ERROR) - { - rust_error_at (mappings->lookup_location (id), - "failure in type resolution for %u", id); - return false; - } - // nothing to do if (ty->get_kind () != TyTy::TypeKind::INFER) return true; - TyTy::InferType *infer_var = (TyTy::InferType *) ty; + TyTy::InferType *infer_var = static_cast<TyTy::InferType *> (ty); TyTy::BaseType *default_type; bool ok = infer_var->default_type (&default_type); if (!ok) diff --git a/gcc/rust/typecheck/rust-tyty-rules.h b/gcc/rust/typecheck/rust-tyty-rules.h index f95e7bf..1dd0112 100644 --- a/gcc/rust/typecheck/rust-tyty-rules.h +++ b/gcc/rust/typecheck/rust-tyty-rules.h @@ -89,6 +89,11 @@ public: for (auto ref : other->get_combined_refs ()) resolved->append_reference (ref); + other->append_reference (resolved->get_ref ()); + other->append_reference (get_base ()->get_ref ()); + get_base ()->append_reference (resolved->get_ref ()); + get_base ()->append_reference (other->get_ref ()); + bool result_resolved = resolved->get_kind () != TyTy::TypeKind::INFER; bool result_is_infer_var = resolved->get_kind () == TyTy::TypeKind::INFER; bool results_is_non_general_infer_var diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index d9a4243..847ca88 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -263,6 +263,7 @@ TyVar::get_implicit_infer_var (Location locus) infer); mappings->insert_location (mappings->get_current_crate (), infer->get_ref (), locus); + return TyVar (infer->get_ref ()); } @@ -341,8 +342,34 @@ InferType::cast (BaseType *other) BaseType * InferType::clone () const { - return new InferType (get_ref (), get_ty_ref (), get_infer_kind (), - get_ident ().locus, get_combined_refs ()); + // clones for inference variables are special in that they _must_ exist within + // the type check context and we must ensure we don't loose the chain + // otherwise we will end up in the missing type annotations case + // + // This means we cannot simply take over the same reference we must generate a + // new ref just like the get_implicit_infer_var code then we can setup the + // chain of references accordingly to ensure we don't loose the ability to + // update the inference variables when we solve the type + + auto mappings = Analysis::Mappings::get (); + auto context = Resolver::TypeCheckContext::get (); + + InferType *clone + = new InferType (mappings->get_next_hir_id (), get_infer_kind (), + get_ident ().locus, get_combined_refs ()); + + context->insert_type (Analysis::NodeMapping (mappings->get_current_crate (), + UNKNOWN_NODEID, + clone->get_ref (), + UNKNOWN_LOCAL_DEFID), + clone); + mappings->insert_location (mappings->get_current_crate (), clone->get_ref (), + mappings->lookup_location (get_ref ())); + + // setup the chain to reference this + clone->append_reference (get_ref ()); + + return clone; } bool diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index 1f157c8..271ce2c 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -955,12 +955,23 @@ public: BaseType *infer_substitions (Location locus) { std::vector<SubstitutionArg> args; + std::map<std::string, BaseType *> argument_mappings; for (auto &p : get_substs ()) { if (p.needs_substitution ()) { - TyVar infer_var = TyVar::get_implicit_infer_var (locus); - args.push_back (SubstitutionArg (&p, infer_var.get_tyty ())); + const std::string &symbol = p.get_param_ty ()->get_symbol (); + auto it = argument_mappings.find (symbol); + if (it == argument_mappings.end ()) + { + TyVar infer_var = TyVar::get_implicit_infer_var (locus); + args.push_back (SubstitutionArg (&p, infer_var.get_tyty ())); + argument_mappings[symbol] = infer_var.get_tyty (); + } + else + { + args.push_back (SubstitutionArg (&p, it->second)); + } } else { diff --git a/gcc/testsuite/rust/compile/torture/issue-893-2.rs b/gcc/testsuite/rust/compile/torture/issue-893-2.rs index c0eb1bb..88a865d 100644 --- a/gcc/testsuite/rust/compile/torture/issue-893-2.rs +++ b/gcc/testsuite/rust/compile/torture/issue-893-2.rs @@ -24,7 +24,7 @@ impl Baz<i32, f32> { pub fn main() { let a = Foo::<i32>::new::<f32>(123, 456f32); - // let b = Foo::new::<f32>(123, 456f32); + let b = Foo::new::<f32>(123, 456f32); let c = Bar::<i32>(123); let d = Bar::baz(c); |