Skip to content

Commit fe13ad4

Browse files
bors[bot]philberty
andauthored
Merge #1030
1030: Rewrite our unconstrained type-param error checking r=philberty a=philberty This is a series of patches that were all required to fix this issue. We now take advantage of our substitutions abstractions and traits so that our TypeBoundPredicate's which form the basis of our HRTB code I think this class is almost akin to rustc existential-trait-references. This now reuses the same code path to give us the same error checking for generics as we get with ADT's, functions etc. With this refactoring in place we can then reuse the abstractions to map the ID's from the used arguments in the type-bound-predicate, the impl block type substation mappings and the self type itself. There are quite a few cases to handle and our testsuite picked up all the regressions so no behaviour of our existing test-cases have changed now. See each commit for more detailed information. Fixes #1019 Addresses #849 Co-authored-by: Philip Herron <[email protected]>
2 parents bb234b0 + 8086790 commit fe13ad4

15 files changed

+483
-240
lines changed

gcc/rust/Make-lang.in

+1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ GRS_OBJS = \
9999
rust/rust-hir-type-check-pattern.o \
100100
rust/rust-hir-type-check-expr.o \
101101
rust/rust-hir-dot-operator.o \
102+
rust/rust-hir-type-check-base.o \
102103
rust/rust-autoderef.o \
103104
rust/rust-substitution-mapper.o \
104105
rust/rust-lint-marklive.o \

gcc/rust/hir/tree/rust-hir-path.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,7 @@ struct GenericArgs
133133

134134
GenericArgs (std::vector<Lifetime> lifetime_args,
135135
std::vector<std::unique_ptr<Type> > type_args,
136-
std::vector<GenericArgsBinding> binding_args,
137-
Location locus = Location ())
136+
std::vector<GenericArgsBinding> binding_args, Location locus)
138137
: lifetime_args (std::move (lifetime_args)),
139138
type_args (std::move (type_args)),
140139
binding_args (std::move (binding_args)), locus (locus)
@@ -471,7 +470,7 @@ class TypePathSegmentGeneric : public TypePathSegment
471470
has_separating_scope_resolution, locus),
472471
generic_args (GenericArgs (std::move (lifetime_args),
473472
std::move (type_args),
474-
std::move (binding_args)))
473+
std::move (binding_args), locus))
475474
{}
476475

477476
std::string as_string () const override;

gcc/rust/typecheck/rust-hir-trait-ref.h

+34-5
Original file line numberDiff line numberDiff line change
@@ -181,27 +181,48 @@ class TraitReference
181181
public:
182182
TraitReference (const HIR::Trait *hir_trait_ref,
183183
std::vector<TraitItemReference> item_refs,
184-
std::vector<const TraitReference *> super_traits)
184+
std::vector<const TraitReference *> super_traits,
185+
std::vector<TyTy::SubstitutionParamMapping> substs)
185186
: hir_trait_ref (hir_trait_ref), item_refs (item_refs),
186187
super_traits (super_traits)
187-
{}
188+
{
189+
trait_substs.clear ();
190+
trait_substs.reserve (substs.size ());
191+
for (const auto &p : substs)
192+
trait_substs.push_back (p.clone ());
193+
}
188194

189195
TraitReference (TraitReference const &other)
190-
: hir_trait_ref (other.hir_trait_ref), item_refs (other.item_refs)
191-
{}
196+
: hir_trait_ref (other.hir_trait_ref), item_refs (other.item_refs),
197+
super_traits (other.super_traits)
198+
{
199+
trait_substs.clear ();
200+
trait_substs.reserve (other.trait_substs.size ());
201+
for (const auto &p : other.trait_substs)
202+
trait_substs.push_back (p.clone ());
203+
}
192204

193205
TraitReference &operator= (TraitReference const &other)
194206
{
195207
hir_trait_ref = other.hir_trait_ref;
196208
item_refs = other.item_refs;
209+
super_traits = other.super_traits;
210+
211+
trait_substs.clear ();
212+
trait_substs.reserve (other.trait_substs.size ());
213+
for (const auto &p : other.trait_substs)
214+
trait_substs.push_back (p.clone ());
197215

198216
return *this;
199217
}
200218

201219
TraitReference (TraitReference &&other) = default;
202220
TraitReference &operator= (TraitReference &&other) = default;
203221

204-
static TraitReference error () { return TraitReference (nullptr, {}, {}); }
222+
static TraitReference error ()
223+
{
224+
return TraitReference (nullptr, {}, {}, {});
225+
}
205226

206227
bool is_error () const { return hir_trait_ref == nullptr; }
207228

@@ -384,10 +405,18 @@ class TraitReference
384405
return is_safe;
385406
}
386407

408+
bool trait_has_generics () const { return !trait_substs.empty (); }
409+
410+
std::vector<TyTy::SubstitutionParamMapping> get_trait_substs () const
411+
{
412+
return trait_substs;
413+
}
414+
387415
private:
388416
const HIR::Trait *hir_trait_ref;
389417
std::vector<TraitItemReference> item_refs;
390418
std::vector<const TraitReference *> super_traits;
419+
std::vector<TyTy::SubstitutionParamMapping> trait_substs;
391420
};
392421

393422
class AssociatedImplTrait

gcc/rust/typecheck/rust-hir-trait-resolve.h

+9-3
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,13 @@ class TraitResolver : public TypeCheckBase
150150

151151
// They also inherit themselves as a bound this enables a trait item to
152152
// reference other Self::trait_items
153+
std::vector<TyTy::SubstitutionParamMapping> self_subst_copy;
154+
for (auto &sub : substitutions)
155+
self_subst_copy.push_back (sub.clone ());
156+
153157
specified_bounds.push_back (
154158
TyTy::TypeBoundPredicate (trait_reference->get_mappings ().get_defid (),
159+
std::move (self_subst_copy),
155160
trait_reference->get_locus ()));
156161

157162
std::vector<const TraitReference *> super_traits;
@@ -168,8 +173,8 @@ class TraitResolver : public TypeCheckBase
168173
// FIXME this might be recursive we need a check for that
169174

170175
TraitReference *trait = resolve_trait_path (b->get_path ());
171-
TyTy::TypeBoundPredicate predicate (
172-
trait->get_mappings ().get_defid (), bound->get_locus ());
176+
TyTy::TypeBoundPredicate predicate (*trait,
177+
bound->get_locus ());
173178

174179
specified_bounds.push_back (std::move (predicate));
175180
super_traits.push_back (predicate.get ());
@@ -193,7 +198,8 @@ class TraitResolver : public TypeCheckBase
193198
}
194199

195200
TraitReference trait_object (trait_reference, item_refs,
196-
std::move (super_traits));
201+
std::move (super_traits),
202+
std::move (substitutions));
197203
context->insert_trait_reference (
198204
trait_reference->get_mappings ().get_defid (), std::move (trait_object));
199205

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (C) 2020-2022 Free Software Foundation, Inc.
2+
3+
// This file is part of GCC.
4+
5+
// GCC is free software; you can redistribute it and/or modify it under
6+
// the terms of the GNU General Public License as published by the Free
7+
// Software Foundation; either version 3, or (at your option) any later
8+
// version.
9+
10+
// GCC is distributed in the hope that it will be useful, but WITHOUT ANY
11+
// WARRANTY; without even the implied warranty of MERCHANTABILITY or
12+
// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
13+
// for more details.
14+
15+
// You should have received a copy of the GNU General Public License
16+
// along with GCC; see the file COPYING3. If not see
17+
// <http://www.gnu.org/licenses/>.
18+
19+
#include "rust-hir-type-check-base.h"
20+
21+
namespace Rust {
22+
namespace Resolver {
23+
24+
bool
25+
TypeCheckBase::check_for_unconstrained (
26+
const std::vector<TyTy::SubstitutionParamMapping> &params_to_constrain,
27+
const TyTy::SubstitutionArgumentMappings &constraint_a,
28+
const TyTy::SubstitutionArgumentMappings &constraint_b,
29+
const TyTy::BaseType *reference)
30+
{
31+
std::set<HirId> symbols_to_constrain;
32+
std::map<HirId, Location> symbol_to_location;
33+
for (const auto &p : params_to_constrain)
34+
{
35+
HirId ref = p.get_param_ty ()->get_ref ();
36+
symbols_to_constrain.insert (ref);
37+
symbol_to_location.insert ({ref, p.get_param_locus ()});
38+
}
39+
40+
// set up the set of constrained symbols
41+
std::set<HirId> constrained_symbols;
42+
for (const auto &c : constraint_a.get_mappings ())
43+
{
44+
const TyTy::BaseType *arg = c.get_tyty ();
45+
if (arg != nullptr)
46+
{
47+
const TyTy::BaseType *p = arg->get_root ();
48+
constrained_symbols.insert (p->get_ty_ref ());
49+
}
50+
}
51+
for (const auto &c : constraint_b.get_mappings ())
52+
{
53+
const TyTy::BaseType *arg = c.get_tyty ();
54+
if (arg != nullptr)
55+
{
56+
const TyTy::BaseType *p = arg->get_root ();
57+
constrained_symbols.insert (p->get_ty_ref ());
58+
}
59+
}
60+
61+
const auto root = reference->get_root ();
62+
if (root->get_kind () == TyTy::TypeKind::PARAM)
63+
{
64+
const TyTy::ParamType *p = static_cast<const TyTy::ParamType *> (root);
65+
constrained_symbols.insert (p->get_ty_ref ());
66+
}
67+
68+
// check for unconstrained
69+
bool unconstrained = false;
70+
for (auto &sym : symbols_to_constrain)
71+
{
72+
bool used = constrained_symbols.find (sym) != constrained_symbols.end ();
73+
if (!used)
74+
{
75+
Location locus = symbol_to_location.at (sym);
76+
rust_error_at (locus, "unconstrained type parameter");
77+
unconstrained = true;
78+
}
79+
}
80+
return unconstrained;
81+
}
82+
83+
} // namespace Resolver
84+
} // namespace Rust

gcc/rust/typecheck/rust-hir-type-check-base.h

+8
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ class TypeCheckBase : public HIR::HIRFullVisitorBase
4747

4848
TraitReference *resolve_trait_path (HIR::TypePath &);
4949

50+
TyTy::TypeBoundPredicate get_predicate_from_bound (HIR::TypePath &path);
51+
52+
bool check_for_unconstrained (
53+
const std::vector<TyTy::SubstitutionParamMapping> &params_to_constrain,
54+
const TyTy::SubstitutionArgumentMappings &constraint_a,
55+
const TyTy::SubstitutionArgumentMappings &constraint_b,
56+
const TyTy::BaseType *reference);
57+
5058
Analysis::Mappings *mappings;
5159
Resolver *resolver;
5260
TypeCheckContext *context;

gcc/rust/typecheck/rust-hir-type-check-item.h

+19-24
Original file line numberDiff line numberDiff line change
@@ -70,34 +70,15 @@ class TypeCheckItem : public TypeCheckBase
7070
}
7171
}
7272

73-
std::vector<TyTy::TypeBoundPredicate> specified_bounds;
73+
auto specified_bound = TyTy::TypeBoundPredicate::error ();
7474
TraitReference *trait_reference = &TraitReference::error_node ();
7575
if (impl_block.has_trait_ref ())
7676
{
7777
std::unique_ptr<HIR::TypePath> &ref = impl_block.get_trait_ref ();
7878
trait_reference = TraitResolver::Resolve (*ref.get ());
7979
rust_assert (!trait_reference->is_error ());
8080

81-
// setup the bound
82-
TyTy::TypeBoundPredicate predicate (
83-
trait_reference->get_mappings ().get_defid (), ref->get_locus ());
84-
auto &final_seg = ref->get_final_segment ();
85-
if (final_seg->is_generic_segment ())
86-
{
87-
auto final_generic_seg
88-
= static_cast<HIR::TypePathSegmentGeneric *> (final_seg.get ());
89-
if (final_generic_seg->has_generic_args ())
90-
{
91-
HIR::GenericArgs &generic_args
92-
= final_generic_seg->get_generic_args ();
93-
94-
// this is applying generic arguments to a trait
95-
// reference
96-
predicate.apply_generic_arguments (&generic_args);
97-
}
98-
}
99-
100-
specified_bounds.push_back (std::move (predicate));
81+
specified_bound = get_predicate_from_bound (*ref.get ());
10182
}
10283

10384
TyTy::BaseType *self = nullptr;
@@ -108,11 +89,25 @@ class TypeCheckItem : public TypeCheckBase
10889
"failed to resolve Self for ImplBlock");
10990
return;
11091
}
111-
// inherit the bounds
112-
self->inherit_bounds (specified_bounds);
11392

93+
// inherit the bounds
94+
if (!specified_bound.is_error ())
95+
self->inherit_bounds ({specified_bound});
96+
97+
// check for any unconstrained type-params
98+
const TyTy::SubstitutionArgumentMappings trait_constraints
99+
= specified_bound.get_substitution_arguments ();
100+
const TyTy::SubstitutionArgumentMappings impl_constraints
101+
= GetUsedSubstArgs::From (self);
102+
103+
bool impl_block_has_unconstrained_typarams
104+
= check_for_unconstrained (substitutions, trait_constraints,
105+
impl_constraints, self);
106+
if (impl_block_has_unconstrained_typarams)
107+
return;
108+
109+
// validate the impl items
114110
bool is_trait_impl_block = !trait_reference->is_error ();
115-
116111
std::vector<const TraitItemReference *> trait_item_refs;
117112
for (auto &impl_item : impl_block.get_impl_items ())
118113
{

gcc/rust/typecheck/rust-hir-type-check-toplevel.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -468,9 +468,8 @@ class TypeCheckTopLevel : public TypeCheckBase
468468
ResolveWhereClauseItem::Resolve (*where_clause_item.get ());
469469
}
470470

471-
auto self
472-
= TypeCheckType::Resolve (impl_block.get_type ().get (), &substitutions);
473-
if (self == nullptr || self->get_kind () == TyTy::TypeKind::ERROR)
471+
auto self = TypeCheckType::Resolve (impl_block.get_type ().get ());
472+
if (self->get_kind () == TyTy::TypeKind::ERROR)
474473
return;
475474

476475
for (auto &impl_item : impl_block.get_impl_items ())

gcc/rust/typecheck/rust-hir-type-check-type.cc

+4-25
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,6 @@ TypeCheckType::visit (HIR::TypePath &path)
8484
}
8585

8686
translated = SubstMapper::Resolve (path_type, path.get_locus (), &args);
87-
if (translated->get_kind () != TyTy::TypeKind::ERROR
88-
&& mappings != nullptr)
89-
{
90-
check_for_unconstrained (args.get_type_args ());
91-
}
9287
}
9388
else if (!args.is_empty ())
9489
{
@@ -548,27 +543,11 @@ TypeCheckType::visit (HIR::TraitObjectType &type)
548543
HIR::TypeParamBound &b = *bound.get ();
549544
HIR::TraitBound &trait_bound = static_cast<HIR::TraitBound &> (b);
550545

551-
auto &type_path = trait_bound.get_path ();
552-
TraitReference *trait = resolve_trait_path (type_path);
553-
TyTy::TypeBoundPredicate predicate (trait->get_mappings ().get_defid (),
554-
trait_bound.get_locus ());
555-
auto &final_seg = type_path.get_final_segment ();
556-
if (final_seg->is_generic_segment ())
557-
{
558-
auto final_generic_seg
559-
= static_cast<HIR::TypePathSegmentGeneric *> (final_seg.get ());
560-
if (final_generic_seg->has_generic_args ())
561-
{
562-
HIR::GenericArgs &generic_args
563-
= final_generic_seg->get_generic_args ();
564-
565-
// this is applying generic arguments to a trait
566-
// reference
567-
predicate.apply_generic_arguments (&generic_args);
568-
}
569-
}
546+
TyTy::TypeBoundPredicate predicate
547+
= get_predicate_from_bound (trait_bound.get_path ());
570548

571-
if (predicate.is_object_safe (true, type.get_locus ()))
549+
if (!predicate.is_error ()
550+
&& predicate.is_object_safe (true, type.get_locus ()))
572551
specified_bounds.push_back (std::move (predicate));
573552
}
574553

0 commit comments

Comments
 (0)