[gcc r14-7518] gccrs: Track Self properly with TypePredicateBounds
Arthur Cohen
cohenarthur@gcc.gnu.org
Tue Jan 16 17:40:51 GMT 2024
https://gcc.gnu.org/g:b39ecbfec93867596dac5914d332b3e3c790a0fd
commit r14-7518-gb39ecbfec93867596dac5914d332b3e3c790a0fd
Author: Philip Herron <herron.philip@googlemail.com>
Date: Tue Apr 18 17:56:43 2023 +0100
gccrs: Track Self properly with TypePredicateBounds
When we handle generic trait bounds we never tracked its associated Self
type. Its important to remember a Trait Predicate is associated with a type
this means we end up missing a lot of helpful type information down the
line relating to higher ranked trait bounds and associated types
compuations. Remember traits have an implict Generic Type Parameter of Self
we use this in computing trait defintions so in that case no associated
type is specified which is to be expected but in all other cases we do
even if it is generic its still useful type information to keep track of.
There is one regression here with compile/issue-1893.rs this testcase
mostly worked out as a fluke rather than a proper fix so its no suprise
here it has regressed the other two test cases one where the number
generic arguments has changed, Rustc gets around this and has a seperate
error message for this case.
We need to solve this patch in order to solve #2019
gcc/rust/ChangeLog:
* typecheck/rust-hir-trait-resolve.cc (TraitResolver::resolve_trait): use new interface
* typecheck/rust-hir-type-check-base.h: update prototype to include Self
* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): lifewise
* typecheck/rust-hir-type-check-item.cc (TypeCheckItem::resolve_impl_block_substitutions):
likewise
* typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::visit): likewise
(TypeCheckExpr::resolve_segments): likewise
* typecheck/rust-hir-type-check-type.cc (TypeCheckType::visit): likewise
(TypeResolveGenericParam::visit): likewise
(ResolveWhereClauseItem::visit): likewise
* typecheck/rust-tyty-bounds.cc (TypeCheckBase::get_predicate_from_bound): likewise
(TypeBoundPredicate::apply_generic_arguments): likewise
(TypeBoundsMappings::lookup_predicate): likewise
* typecheck/rust-tyty-bounds.h: likewise
* typecheck/rust-tyty.h:likewise
gcc/testsuite/ChangeLog:
* rust/compile/issue-1893.rs: regression
* rust/compile/traits12.rs: rustc uses a custom error message here
* rust/compile/unconstrained_type_param.rs: extra error message
Signed-off-by: Philip Herron <herron.philip@googlemail.com>
Diff:
---
gcc/rust/typecheck/rust-hir-trait-resolve.cc | 28 +++++++----
gcc/rust/typecheck/rust-hir-type-check-base.h | 3 +-
gcc/rust/typecheck/rust-hir-type-check-expr.cc | 2 +-
gcc/rust/typecheck/rust-hir-type-check-item.cc | 6 ++-
gcc/rust/typecheck/rust-hir-type-check-path.cc | 55 +++++++++++++---------
gcc/rust/typecheck/rust-hir-type-check-type.cc | 40 ++++++++++++++--
gcc/rust/typecheck/rust-tyty-bounds.cc | 49 ++++++++++++++++---
gcc/rust/typecheck/rust-tyty-bounds.h | 3 ++
gcc/rust/typecheck/rust-tyty.h | 3 +-
gcc/testsuite/rust/compile/issue-1893.rs | 1 +
gcc/testsuite/rust/compile/traits12.rs | 2 +-
.../rust/compile/unconstrained_type_param.rs | 5 +-
12 files changed, 148 insertions(+), 49 deletions(-)
diff --git a/gcc/rust/typecheck/rust-hir-trait-resolve.cc b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
index 48ffd088542..20ee2e25306 100644
--- a/gcc/rust/typecheck/rust-hir-trait-resolve.cc
+++ b/gcc/rust/typecheck/rust-hir-trait-resolve.cc
@@ -228,7 +228,9 @@ TraitResolver::resolve_trait (HIR::Trait *trait_reference)
HIR::TraitBound *b
= static_cast<HIR::TraitBound *> (bound.get ());
- auto predicate = get_predicate_from_bound (b->get_path ());
+ auto predicate = get_predicate_from_bound (
+ b->get_path (),
+ nullptr /*this will setup a PLACEHOLDER for self*/);
if (predicate.is_error ())
return &TraitReference::error_node ();
@@ -484,26 +486,36 @@ AssociatedImplTrait::setup_associated_types (
// infer the arguments on the predicate
std::vector<TyTy::BaseType *> impl_trait_predicate_args;
- for (const auto &arg : impl_predicate.get_substs ())
+
+ for (size_t i = 0; i < impl_predicate.get_substs ().size (); i++)
{
- const TyTy::ParamType *p = arg.get_param_ty ();
- if (p->get_symbol ().compare ("Self") == 0)
+ const auto &arg = impl_predicate.get_substs ().at (i);
+ if (i == 0)
continue;
+ const TyTy::ParamType *p = arg.get_param_ty ();
TyTy::BaseType *r = p->resolve ();
- r = SubstMapperInternal::Resolve (r, infer_arguments);
+ if (!r->is_concrete ())
+ {
+ r = SubstMapperInternal::Resolve (r, infer_arguments);
+ }
impl_trait_predicate_args.push_back (r);
}
// unify the bounds arguments
std::vector<TyTy::BaseType *> hrtb_bound_arguments;
- for (const auto &arg : bound.get_substs ())
+ for (size_t i = 0; i < bound.get_substs ().size (); i++)
{
- const TyTy::ParamType *p = arg.get_param_ty ();
- if (p->get_symbol ().compare ("Self") == 0)
+ const auto &arg = bound.get_substs ().at (i);
+ if (i == 0)
continue;
+ const TyTy::ParamType *p = arg.get_param_ty ();
TyTy::BaseType *r = p->resolve ();
+ if (!r->is_concrete ())
+ {
+ r = SubstMapperInternal::Resolve (r, infer_arguments);
+ }
hrtb_bound_arguments.push_back (r);
}
diff --git a/gcc/rust/typecheck/rust-hir-type-check-base.h b/gcc/rust/typecheck/rust-hir-type-check-base.h
index 5b654b616aa..f73b61b83e0 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-base.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-base.h
@@ -37,7 +37,8 @@ protected:
TraitReference *resolve_trait_path (HIR::TypePath &);
- TyTy::TypeBoundPredicate get_predicate_from_bound (HIR::TypePath &path);
+ TyTy::TypeBoundPredicate
+ get_predicate_from_bound (HIR::TypePath &path, HIR::Type *associated_self);
bool check_for_unconstrained (
const std::vector<TyTy::SubstitutionParamMapping> ¶ms_to_constrain,
diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.cc b/gcc/rust/typecheck/rust-hir-type-check-expr.cc
index 4e4fdbaec1e..f82204872c1 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-expr.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-expr.cc
@@ -1572,7 +1572,7 @@ TypeCheckExpr::visit (HIR::ClosureExpr &expr)
args.get_type_args ().push_back (std::unique_ptr<HIR::Type> (implicit_tuple));
// apply the arguments
- predicate.apply_generic_arguments (&args);
+ predicate.apply_generic_arguments (&args, false);
// finally inherit the trait bound
infered->inherit_bounds ({predicate});
diff --git a/gcc/rust/typecheck/rust-hir-type-check-item.cc b/gcc/rust/typecheck/rust-hir-type-check-item.cc
index 2116a9bdabc..5ffebe91a6c 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-item.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-item.cc
@@ -501,7 +501,8 @@ TypeCheckItem::resolve_impl_block_substitutions (HIR::ImplBlock &impl_block,
// we don't error out here see: gcc/testsuite/rust/compile/traits2.rs
// for example
- specified_bound = get_predicate_from_bound (*ref.get ());
+ specified_bound
+ = get_predicate_from_bound (*ref.get (), impl_block.get_type ().get ());
}
TyTy::BaseType *self = TypeCheckType::Resolve (impl_block.get_type ().get ());
@@ -537,7 +538,8 @@ TypeCheckItem::validate_trait_impl_block (
// we don't error out here see: gcc/testsuite/rust/compile/traits2.rs
// for example
- specified_bound = get_predicate_from_bound (*ref.get ());
+ specified_bound
+ = get_predicate_from_bound (*ref.get (), impl_block.get_type ().get ());
}
bool is_trait_impl_block = !trait_reference->is_error ();
diff --git a/gcc/rust/typecheck/rust-hir-type-check-path.cc b/gcc/rust/typecheck/rust-hir-type-check-path.cc
index c24a1bd37cd..dd7bc9aee6e 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-path.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-path.cc
@@ -18,6 +18,7 @@
#include "rust-hir-type-check-expr.h"
#include "rust-hir-type-check-type.h"
+#include "rust-hir-type-check-item.h"
#include "rust-hir-trait-resolve.h"
#include "rust-substitution-mapper.h"
#include "rust-hir-path-probe.h"
@@ -59,7 +60,9 @@ TypeCheckExpr::visit (HIR::QualifiedPathInExpression &expr)
return;
// get the predicate for the bound
- auto specified_bound = get_predicate_from_bound (*trait_path_ref.get ());
+ auto specified_bound
+ = get_predicate_from_bound (*trait_path_ref.get (),
+ qual_path_type.get_type ().get ());
if (specified_bound.is_error ())
return;
@@ -396,33 +399,41 @@ TypeCheckExpr::resolve_segments (NodeId root_resolved_node_id,
bool found_impl_trait
= context->lookup_associated_trait_impl (impl_block_id,
&associated);
- TyTy::BaseType *impl_block_ty = nullptr;
- if (found_impl_trait)
- {
- TyTy::TypeBoundPredicate predicate (*associated->get_trait (),
- seg.get_locus ());
- impl_block_ty
- = associated->setup_associated_types (prev_segment, predicate);
- }
- else
- {
- // get the type of the parent Self
- HirId impl_ty_id = associated_impl_block->get_type ()
- ->get_mappings ()
- .get_hirid ();
-
- bool ok = query_type (impl_ty_id, &impl_block_ty);
- rust_assert (ok);
+ TyTy::BaseType *impl_block_ty
+ = TypeCheckItem::ResolveImplBlockSelf (*associated_impl_block);
- if (impl_block_ty->needs_generic_substitutions ())
- impl_block_ty
- = SubstMapper::InferSubst (impl_block_ty, seg.get_locus ());
- }
+ if (impl_block_ty->needs_generic_substitutions ())
+ impl_block_ty
+ = SubstMapper::InferSubst (impl_block_ty, seg.get_locus ());
prev_segment = unify_site (seg.get_mappings ().get_hirid (),
TyTy::TyWithLocation (prev_segment),
TyTy::TyWithLocation (impl_block_ty),
seg.get_locus ());
+
+ if (found_impl_trait)
+ {
+ // we need to setup with apropriate bounds
+ HIR::TypePath &bound_path
+ = *associated->get_impl_block ()->get_trait_ref ().get ();
+
+ // generate an implicit HIR Type we can apply to the predicate
+ HirId implicit_id = mappings->get_next_hir_id ();
+ context->insert_implicit_type (implicit_id, impl_block_ty);
+
+ Analysis::NodeMapping mappings (expr_mappings.get_crate_num (),
+ expr_mappings.get_nodeid (),
+ implicit_id,
+ expr_mappings.get_local_defid ());
+ HIR::TypePath *implicit_self_bound
+ = new HIR::TypePath (mappings, {},
+ Linemap::predeclared_location (), false);
+
+ TyTy::TypeBoundPredicate predicate
+ = get_predicate_from_bound (bound_path, implicit_self_bound);
+ impl_block_ty
+ = associated->setup_associated_types (prev_segment, predicate);
+ }
}
if (tyseg->needs_generic_substitutions ())
diff --git a/gcc/rust/typecheck/rust-hir-type-check-type.cc b/gcc/rust/typecheck/rust-hir-type-check-type.cc
index c78ecb51fd4..41c444029b6 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-type.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-type.cc
@@ -193,7 +193,8 @@ TypeCheckType::visit (HIR::QualifiedPathInType &path)
// get the predicate for the bound
auto specified_bound
- = get_predicate_from_bound (*qual_path_type.get_trait ().get ());
+ = get_predicate_from_bound (*qual_path_type.get_trait ().get (),
+ qual_path_type.get_type ().get ());
if (specified_bound.is_error ())
return;
@@ -559,8 +560,9 @@ TypeCheckType::visit (HIR::TraitObjectType &type)
HIR::TypeParamBound &b = *bound.get ();
HIR::TraitBound &trait_bound = static_cast<HIR::TraitBound &> (b);
- TyTy::TypeBoundPredicate predicate
- = get_predicate_from_bound (trait_bound.get_path ());
+ TyTy::TypeBoundPredicate predicate = get_predicate_from_bound (
+ trait_bound.get_path (),
+ nullptr /*this will setup a PLACEHOLDER for self*/);
if (!predicate.is_error ()
&& predicate.is_object_safe (true, type.get_locus ()))
@@ -682,6 +684,30 @@ TypeResolveGenericParam::visit (HIR::TypeParam ¶m)
if (param.has_type ())
TypeCheckType::Resolve (param.get_type ().get ());
+ HIR::Type *implicit_self_bound = nullptr;
+ if (param.has_type_param_bounds ())
+ {
+ // We need two possible parameter types. One with no Bounds and one with
+ // the bounds. the Self type for the bounds cannot itself contain the
+ // bounds otherwise it will be a trait cycle
+ HirId implicit_id = mappings->get_next_hir_id ();
+ TyTy::ParamType *p
+ = new TyTy::ParamType (param.get_type_representation (),
+ param.get_locus (), implicit_id, param,
+ {} /*empty specified bounds*/);
+ context->insert_implicit_type (implicit_id, p);
+
+ // generate an implicit HIR Type we can apply to the predicate
+ Analysis::NodeMapping mappings (param.get_mappings ().get_crate_num (),
+ param.get_mappings ().get_nodeid (),
+ implicit_id,
+ param.get_mappings ().get_local_defid ());
+ implicit_self_bound
+ = new HIR::TypePath (mappings, {}, Linemap::predeclared_location (),
+ false);
+ }
+
+ // resolve the bounds
std::vector<TyTy::TypeBoundPredicate> specified_bounds;
if (param.has_type_param_bounds ())
{
@@ -694,7 +720,8 @@ TypeResolveGenericParam::visit (HIR::TypeParam ¶m)
= static_cast<HIR::TraitBound *> (bound.get ());
TyTy::TypeBoundPredicate predicate
- = get_predicate_from_bound (b->get_path ());
+ = get_predicate_from_bound (b->get_path (),
+ implicit_self_bound);
if (!predicate.is_error ())
specified_bounds.push_back (std::move (predicate));
}
@@ -738,6 +765,8 @@ ResolveWhereClauseItem::visit (HIR::TypeBoundWhereClauseItem &item)
auto &binding_type_path = item.get_bound_type ();
TyTy::BaseType *binding = TypeCheckType::Resolve (binding_type_path.get ());
+ // FIXME double check there might be a trait cycle here see TypeParam handling
+
std::vector<TyTy::TypeBoundPredicate> specified_bounds;
for (auto &bound : item.get_type_param_bounds ())
{
@@ -747,7 +776,8 @@ ResolveWhereClauseItem::visit (HIR::TypeBoundWhereClauseItem &item)
HIR::TraitBound *b = static_cast<HIR::TraitBound *> (bound.get ());
TyTy::TypeBoundPredicate predicate
- = get_predicate_from_bound (b->get_path ());
+ = get_predicate_from_bound (b->get_path (),
+ binding_type_path.get ());
if (!predicate.is_error ())
specified_bounds.push_back (std::move (predicate));
}
diff --git a/gcc/rust/typecheck/rust-tyty-bounds.cc b/gcc/rust/typecheck/rust-tyty-bounds.cc
index 6977bee2f3f..23a2926faeb 100644
--- a/gcc/rust/typecheck/rust-tyty-bounds.cc
+++ b/gcc/rust/typecheck/rust-tyty-bounds.cc
@@ -171,7 +171,8 @@ TypeCheckBase::resolve_trait_path (HIR::TypePath &path)
}
TyTy::TypeBoundPredicate
-TypeCheckBase::get_predicate_from_bound (HIR::TypePath &type_path)
+TypeCheckBase::get_predicate_from_bound (HIR::TypePath &type_path,
+ HIR::Type *associated_self)
{
TyTy::TypeBoundPredicate lookup = TyTy::TypeBoundPredicate::error ();
bool already_resolved
@@ -251,17 +252,33 @@ TypeCheckBase::get_predicate_from_bound (HIR::TypePath &type_path)
break;
}
+ if (associated_self != nullptr)
+ {
+ std::vector<std::unique_ptr<HIR::Type>> type_args;
+ type_args.push_back (
+ std::unique_ptr<HIR::Type> (associated_self->clone_type ()));
+ for (auto &arg : args.get_type_args ())
+ {
+ type_args.push_back (std::unique_ptr<HIR::Type> (arg->clone_type ()));
+ }
+
+ args = HIR::GenericArgs (args.get_lifetime_args (), std::move (type_args),
+ args.get_binding_args (), args.get_const_args (),
+ args.get_locus ());
+ }
+
// we try to apply generic arguments when they are non empty and or when the
// predicate requires them so that we get the relevant Foo expects x number
// arguments but got zero see test case rust/compile/traits12.rs
if (!args.is_empty () || predicate.requires_generic_args ())
{
// this is applying generic arguments to a trait reference
- predicate.apply_generic_arguments (&args);
+ predicate.apply_generic_arguments (&args, associated_self != nullptr);
}
context->insert_resolved_predicate (type_path.get_mappings ().get_hirid (),
predicate);
+
return predicate;
}
@@ -416,12 +433,21 @@ TypeBoundPredicate::is_object_safe (bool emit_error, Location locus) const
}
void
-TypeBoundPredicate::apply_generic_arguments (HIR::GenericArgs *generic_args)
+TypeBoundPredicate::apply_generic_arguments (HIR::GenericArgs *generic_args,
+ bool has_associated_self)
{
- // we need to get the substitutions argument mappings but also remember that
- // we have an implicit Self argument which we must be careful to respect
- rust_assert (!used_arguments.is_empty ());
rust_assert (!substitutions.empty ());
+ if (has_associated_self)
+ {
+ used_arguments = SubstitutionArgumentMappings::empty ();
+ }
+ else
+ {
+ // we need to get the substitutions argument mappings but also remember
+ // that we have an implicit Self argument which we must be careful to
+ // respect
+ rust_assert (!used_arguments.is_empty ());
+ }
// now actually perform a substitution
used_arguments = get_mappings_from_generic_args (*generic_args);
@@ -690,6 +716,17 @@ TypeBoundsMappings::get_specified_bounds () const
return specified_bounds;
}
+TypeBoundPredicate
+TypeBoundsMappings::lookup_predicate (DefId id)
+{
+ for (auto &b : specified_bounds)
+ {
+ if (b.get_id () == id)
+ return b;
+ }
+ return TypeBoundPredicate::error ();
+}
+
size_t
TypeBoundsMappings::num_specified_bounds () const
{
diff --git a/gcc/rust/typecheck/rust-tyty-bounds.h b/gcc/rust/typecheck/rust-tyty-bounds.h
index 38e59551e64..bcd51295345 100644
--- a/gcc/rust/typecheck/rust-tyty-bounds.h
+++ b/gcc/rust/typecheck/rust-tyty-bounds.h
@@ -20,6 +20,7 @@
#define RUST_TYTY_BOUNDS_H
#include "rust-location.h"
+#include "rust-mapping-common.h"
namespace Rust {
@@ -68,6 +69,8 @@ public:
const std::vector<TypeBoundPredicate> &get_specified_bounds () const;
+ TypeBoundPredicate lookup_predicate (DefId id);
+
size_t num_specified_bounds () const;
std::string raw_bounds_as_string () const;
diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h
index 6bbaae7f441..65073da7118 100644
--- a/gcc/rust/typecheck/rust-tyty.h
+++ b/gcc/rust/typecheck/rust-tyty.h
@@ -408,7 +408,8 @@ public:
// https://doc.rust-lang.org/reference/items/traits.html#object-safety
bool is_object_safe (bool emit_error, Location locus) const;
- void apply_generic_arguments (HIR::GenericArgs *generic_args);
+ void apply_generic_arguments (HIR::GenericArgs *generic_args,
+ bool has_associated_self);
bool contains_item (const std::string &search) const;
diff --git a/gcc/testsuite/rust/compile/issue-1893.rs b/gcc/testsuite/rust/compile/issue-1893.rs
index ff8bef79bbc..8d32e0d37c8 100644
--- a/gcc/testsuite/rust/compile/issue-1893.rs
+++ b/gcc/testsuite/rust/compile/issue-1893.rs
@@ -1,3 +1,4 @@
+// { dg-additional-options "-frust-compile-until=nameresolution" }
pub enum Option<T> {
None,
Some(T),
diff --git a/gcc/testsuite/rust/compile/traits12.rs b/gcc/testsuite/rust/compile/traits12.rs
index 25e0eb7aaa3..b170692f265 100644
--- a/gcc/testsuite/rust/compile/traits12.rs
+++ b/gcc/testsuite/rust/compile/traits12.rs
@@ -10,7 +10,7 @@ struct Foo<T> {
}
impl<T> A for Foo<usize> {
- // { dg-error "generic item takes at least 1 type arguments but 0 were supplied" "" { target *-*-* } .-1 }
+ // { dg-error "generic item takes at least 2 type arguments but 1 were supplied" "" { target *-*-* } .-1 }
// { dg-error "unconstrained type parameter" "" { target *-*-* } .-2 }
type Output = T;
diff --git a/gcc/testsuite/rust/compile/unconstrained_type_param.rs b/gcc/testsuite/rust/compile/unconstrained_type_param.rs
index 7fe74c2c798..9f3db93a5e7 100644
--- a/gcc/testsuite/rust/compile/unconstrained_type_param.rs
+++ b/gcc/testsuite/rust/compile/unconstrained_type_param.rs
@@ -9,6 +9,7 @@ impl<X, Y> Foo<X> {
fn main() {
let a = Foo::test();
- // { dg-error "Failed to resolve expression of function call" "" { target *-*-* } .-1 }
- // { dg-error "failed to type resolve expression" "" { target *-*-* } .-2 }
+ // { dg-error "expected" "" { target *-*-* } .-1 }
+ // { dg-error "Failed to resolve expression of function call" "" { target *-*-* } .-2 }
+ // { dg-error "failed to type resolve expression" "" { target *-*-* } .-3 }
}
More information about the Gcc-cvs
mailing list