public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/rust/master] Fix regression in fix for #1173
@ 2022-06-08 12:38 Thomas Schwinge
0 siblings, 0 replies; only message in thread
From: Thomas Schwinge @ 2022-06-08 12:38 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:ea38a59ee8329bb7b309400a7de57d53ad7bde31
commit ea38a59ee8329bb7b309400a7de57d53ad7bde31
Author: Philip Herron <philip.herron@embecosm.com>
Date: Tue Apr 26 20:12:58 2022 +0100
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.
Diff:
---
gcc/rust/hir/tree/rust-hir-path.h | 2 +
gcc/rust/typecheck/rust-hir-type-check-expr.h | 8 --
gcc/rust/typecheck/rust-hir-type-check-path.cc | 106 +++++-----------------
gcc/rust/typecheck/rust-hir-type-check.cc | 9 +-
gcc/rust/typecheck/rust-tyty-rules.h | 5 +
gcc/rust/typecheck/rust-tyty.cc | 31 ++++++-
gcc/rust/typecheck/rust-tyty.h | 15 ++-
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 84096ce4c44..aa3e29a99c0 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 630eb601885..5633751c5a0 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 530f630d5ec..9960e76dccb 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 b09ffc76481..ee6663877a5 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 f95e7bf7cd1..1dd011229c2 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 d9a42439e74..847ca88900f 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 1f157c8c52a..271ce2c386b 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 c0eb1bb00bc..88a865d66dc 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);
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-06-08 12:38 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08 12:38 [gcc/devel/rust/master] Fix regression in fix for #1173 Thomas Schwinge
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).