Skip to content

Commit ad9e185

Browse files
committed
Fix bad projection substitution
When we have a Trait such as: ``` pub unsafe trait SliceIndex<T> { type Output; fn index(self, slice: &T) -> &Self::Output; } unsafe impl<T> SliceIndex<[T]> for Range<usize> { type Output = [T]; fn index(self, slice: &[T]) -> &[T] { unsafe { &*self.get_unchecked(slice) } } } ``` When we need to verify that the impl index is compatible fir SliceIndex we get the Type info for the trait-item which is: fn<Self, T> index(self: Self, slice: &T) -> &<placeholder=projection<T>=[T]> This projection gets setup and the types are substituted with Self=Range<usize> and T=[T] which ended up substituting the projection twice resulting in a recursive slice [T=[T]]. In this case the associated type is already setup for the placeholder and does not require generic substitution. This means we added a flag to the substitution generic arguments mappings to handle this case. This patch also addressed memory corruption with the TypeBoundPredicate as part of the debugging of this issue which resulted in a segv when trying to debug the mappings. The issue was the copy constructors needed to update the used argument mappings each time since the substitution param mappings are copied and the addresses no longer exist, valgrind was great here to find this issue. Fixes #1120
1 parent d17e0aa commit ad9e185

File tree

11 files changed

+332
-135
lines changed

11 files changed

+332
-135
lines changed

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,9 @@ class TraitItemReference
137137
// the trait will not be stored in its own map yet
138138
void on_resolved ();
139139

140-
void associated_type_set (TyTy::BaseType *ty);
140+
void associated_type_set (TyTy::BaseType *ty) const;
141141

142-
void associated_type_reset ();
142+
void associated_type_reset () const;
143143

144144
bool is_object_safe () const;
145145

@@ -301,6 +301,24 @@ class TraitReference
301301
return false;
302302
}
303303

304+
bool lookup_trait_item_by_type (const std::string &ident,
305+
TraitItemReference::TraitItemType type,
306+
const TraitItemReference **ref) const
307+
{
308+
for (auto &item : item_refs)
309+
{
310+
if (item.get_trait_item_type () != type)
311+
continue;
312+
313+
if (ident.compare (item.get_identifier ()) == 0)
314+
{
315+
*ref = &item;
316+
return true;
317+
}
318+
}
319+
return false;
320+
}
321+
304322
bool lookup_hir_trait_item (const HIR::TraitItem &item,
305323
const TraitItemReference **ref) const
306324
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ TraitItemReference::resolve_item (HIR::TraitItemFunc &func)
128128
}
129129

130130
void
131-
TraitItemReference::associated_type_set (TyTy::BaseType *ty)
131+
TraitItemReference::associated_type_set (TyTy::BaseType *ty) const
132132
{
133133
rust_assert (get_trait_item_type () == TraitItemType::TYPE);
134134

@@ -141,7 +141,7 @@ TraitItemReference::associated_type_set (TyTy::BaseType *ty)
141141
}
142142

143143
void
144-
TraitItemReference::associated_type_reset ()
144+
TraitItemReference::associated_type_reset () const
145145
{
146146
rust_assert (get_trait_item_type () == TraitItemType::TYPE);
147147

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

Lines changed: 78 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ class TypeCheckImplItemWithTrait : public TypeCheckImplItem
385385
using Rust::Resolver::TypeCheckBase::visit;
386386

387387
public:
388-
static const TraitItemReference *
388+
static TyTy::TypeBoundPredicateItem
389389
Resolve (HIR::ImplBlock *parent, HIR::ImplItem *item, TyTy::BaseType *self,
390-
TraitReference &trait_reference,
390+
TyTy::TypeBoundPredicate &trait_reference,
391391
std::vector<TyTy::SubstitutionParamMapping> substitutions)
392392
{
393393
TypeCheckImplItemWithTrait resolver (parent, self, trait_reference,
@@ -398,38 +398,46 @@ class TypeCheckImplItemWithTrait : public TypeCheckImplItem
398398

399399
void visit (HIR::ConstantItem &constant) override
400400
{
401-
trait_reference.lookup_trait_item_by_type (
401+
// normal resolution of the item
402+
TypeCheckImplItem::visit (constant);
403+
TyTy::BaseType *lookup;
404+
if (!context->lookup_type (constant.get_mappings ().get_hirid (), &lookup))
405+
return;
406+
407+
// map the impl item to the associated trait item
408+
const auto tref = trait_reference.get ();
409+
const TraitItemReference *raw_trait_item = nullptr;
410+
bool found = tref->lookup_trait_item_by_type (
402411
constant.get_identifier (), TraitItemReference::TraitItemType::CONST,
403-
&resolved_trait_item);
412+
&raw_trait_item);
404413

405414
// unknown trait item
406-
if (resolved_trait_item->is_error ())
415+
if (!found || raw_trait_item->is_error ())
407416
{
408417
RichLocation r (constant.get_locus ());
409418
r.add_range (trait_reference.get_locus ());
410419
rust_error_at (r, "constant %<%s%> is not a member of trait %<%s%>",
411420
constant.get_identifier ().c_str (),
412421
trait_reference.get_name ().c_str ());
422+
return;
413423
}
414424

415-
// normal resolution of the item
416-
TypeCheckImplItem::visit (constant);
417-
TyTy::BaseType *lookup;
418-
if (!context->lookup_type (constant.get_mappings ().get_hirid (), &lookup))
419-
return;
420-
if (resolved_trait_item->is_error ())
421-
return;
425+
// get the item from the predicate
426+
resolved_trait_item
427+
= trait_reference.lookup_associated_item (raw_trait_item);
428+
rust_assert (!resolved_trait_item.is_error ());
422429

423430
// merge the attributes
424431
const HIR::TraitItem *hir_trait_item
425-
= resolved_trait_item->get_hir_trait_item ();
432+
= resolved_trait_item.get_raw_item ()->get_hir_trait_item ();
426433
merge_attributes (constant.get_outer_attrs (), *hir_trait_item);
427434

428435
// check the types are compatible
429-
if (!resolved_trait_item->get_tyty ()->can_eq (lookup, true))
436+
auto trait_item_type = resolved_trait_item.get_tyty_for_receiver (self);
437+
if (!trait_item_type->can_eq (lookup, true))
430438
{
431439
RichLocation r (constant.get_locus ());
432-
r.add_range (resolved_trait_item->get_locus ());
440+
r.add_range (resolved_trait_item.get_locus ());
433441

434442
rust_error_at (
435443
r, "constant %<%s%> has an incompatible type for trait %<%s%>",
@@ -440,38 +448,46 @@ class TypeCheckImplItemWithTrait : public TypeCheckImplItem
440448

441449
void visit (HIR::TypeAlias &type) override
442450
{
443-
trait_reference.lookup_trait_item_by_type (
451+
// normal resolution of the item
452+
TypeCheckImplItem::visit (type);
453+
TyTy::BaseType *lookup;
454+
if (!context->lookup_type (type.get_mappings ().get_hirid (), &lookup))
455+
return;
456+
457+
// map the impl item to the associated trait item
458+
const auto tref = trait_reference.get ();
459+
const TraitItemReference *raw_trait_item = nullptr;
460+
bool found = tref->lookup_trait_item_by_type (
444461
type.get_new_type_name (), TraitItemReference::TraitItemType::TYPE,
445-
&resolved_trait_item);
462+
&raw_trait_item);
446463

447464
// unknown trait item
448-
if (resolved_trait_item->is_error ())
465+
if (!found || raw_trait_item->is_error ())
449466
{
450467
RichLocation r (type.get_locus ());
451468
r.add_range (trait_reference.get_locus ());
452469
rust_error_at (r, "type alias %<%s%> is not a member of trait %<%s%>",
453470
type.get_new_type_name ().c_str (),
454471
trait_reference.get_name ().c_str ());
472+
return;
455473
}
456474

457-
// normal resolution of the item
458-
TypeCheckImplItem::visit (type);
459-
TyTy::BaseType *lookup;
460-
if (!context->lookup_type (type.get_mappings ().get_hirid (), &lookup))
461-
return;
462-
if (resolved_trait_item->is_error ())
463-
return;
475+
// get the item from the predicate
476+
resolved_trait_item
477+
= trait_reference.lookup_associated_item (raw_trait_item);
478+
rust_assert (!resolved_trait_item.is_error ());
464479

465480
// merge the attributes
466481
const HIR::TraitItem *hir_trait_item
467-
= resolved_trait_item->get_hir_trait_item ();
482+
= resolved_trait_item.get_raw_item ()->get_hir_trait_item ();
468483
merge_attributes (type.get_outer_attrs (), *hir_trait_item);
469484

470485
// check the types are compatible
471-
if (!resolved_trait_item->get_tyty ()->can_eq (lookup, true))
486+
auto trait_item_type = resolved_trait_item.get_tyty_for_receiver (self);
487+
if (!trait_item_type->can_eq (lookup, true))
472488
{
473489
RichLocation r (type.get_locus ());
474-
r.add_range (resolved_trait_item->get_locus ());
490+
r.add_range (resolved_trait_item.get_locus ());
475491

476492
rust_error_at (
477493
r, "type alias %<%s%> has an incompatible type for trait %<%s%>",
@@ -481,83 +497,63 @@ class TypeCheckImplItemWithTrait : public TypeCheckImplItem
481497

482498
// its actually a projection, since we need a way to actually bind the
483499
// generic substitutions to the type itself
484-
TyTy::ProjectionType *projection = new TyTy::ProjectionType (
485-
type.get_mappings ().get_hirid (), lookup, &trait_reference,
486-
resolved_trait_item->get_mappings ().get_defid (), substitutions);
500+
TyTy::ProjectionType *projection
501+
= new TyTy::ProjectionType (type.get_mappings ().get_hirid (), lookup,
502+
tref,
503+
raw_trait_item->get_mappings ().get_defid (),
504+
substitutions);
487505

488506
context->insert_type (type.get_mappings (), projection);
489-
resolved_trait_item->associated_type_set (projection);
507+
raw_trait_item->associated_type_set (projection);
490508
}
491509

492510
void visit (HIR::Function &function) override
493511
{
494-
// resolved_trait_item = trait_reference.lookup_trait_item (
495-
// function.get_function_name (), TraitItemReference::TraitItemType::FN);
496-
trait_reference.lookup_trait_item_by_type (
497-
function.get_function_name (), TraitItemReference::TraitItemType::FN,
498-
&resolved_trait_item);
512+
// we get the error checking from the base method here
513+
TypeCheckImplItem::visit (function);
514+
TyTy::BaseType *lookup;
515+
if (!context->lookup_type (function.get_mappings ().get_hirid (), &lookup))
516+
return;
517+
518+
// map the impl item to the associated trait item
519+
const auto tref = trait_reference.get ();
520+
const TraitItemReference *raw_trait_item = nullptr;
521+
bool found
522+
= tref->lookup_trait_item_by_type (function.get_function_name (),
523+
TraitItemReference::TraitItemType::FN,
524+
&raw_trait_item);
499525

500526
// unknown trait item
501-
if (resolved_trait_item->is_error ())
527+
if (!found || raw_trait_item->is_error ())
502528
{
503529
RichLocation r (function.get_locus ());
504530
r.add_range (trait_reference.get_locus ());
505531
rust_error_at (r, "method %<%s%> is not a member of trait %<%s%>",
506532
function.get_function_name ().c_str (),
507533
trait_reference.get_name ().c_str ());
534+
return;
508535
}
509536

510-
// we get the error checking from the base method here
511-
TypeCheckImplItem::visit (function);
512-
TyTy::BaseType *lookup;
513-
if (!context->lookup_type (function.get_mappings ().get_hirid (), &lookup))
514-
return;
515-
if (resolved_trait_item->is_error ())
516-
return;
537+
// get the item from the predicate
538+
resolved_trait_item
539+
= trait_reference.lookup_associated_item (raw_trait_item);
540+
rust_assert (!resolved_trait_item.is_error ());
517541

518542
// merge the attributes
519543
const HIR::TraitItem *hir_trait_item
520-
= resolved_trait_item->get_hir_trait_item ();
544+
= resolved_trait_item.get_raw_item ()->get_hir_trait_item ();
521545
merge_attributes (function.get_outer_attrs (), *hir_trait_item);
522546

523-
rust_assert (lookup->get_kind () == TyTy::TypeKind::FNDEF);
524-
rust_assert (resolved_trait_item->get_tyty ()->get_kind ()
525-
== TyTy::TypeKind::FNDEF);
526-
527-
TyTy::FnType *fntype = static_cast<TyTy::FnType *> (lookup);
528-
TyTy::FnType *trait_item_fntype
529-
= static_cast<TyTy::FnType *> (resolved_trait_item->get_tyty ());
530-
531-
// sets substitute self into the trait_item_ref->tyty
532-
TyTy::SubstitutionParamMapping *self_mapping = nullptr;
533-
for (auto &param_mapping : trait_item_fntype->get_substs ())
534-
{
535-
const HIR::TypeParam &type_param = param_mapping.get_generic_param ();
536-
if (type_param.get_type_representation ().compare ("Self") == 0)
537-
{
538-
self_mapping = &param_mapping;
539-
break;
540-
}
541-
}
542-
rust_assert (self_mapping != nullptr);
543-
544-
std::vector<TyTy::SubstitutionArg> mappings;
545-
mappings.push_back (TyTy::SubstitutionArg (self_mapping, self));
546-
547-
TyTy::SubstitutionArgumentMappings implicit_self_substs (
548-
mappings, function.get_locus ());
549-
trait_item_fntype
550-
= trait_item_fntype->handle_substitions (implicit_self_substs);
551-
552547
// check the types are compatible
553-
if (!trait_item_fntype->can_eq (fntype, true))
548+
auto trait_item_type = resolved_trait_item.get_tyty_for_receiver (self);
549+
if (!trait_item_type->can_eq (lookup, true))
554550
{
555551
RichLocation r (function.get_locus ());
556-
r.add_range (resolved_trait_item->get_locus ());
552+
r.add_range (resolved_trait_item.get_locus ());
557553

558554
rust_error_at (
559555
r, "method %<%s%> has an incompatible type for trait %<%s%>",
560-
fntype->get_identifier ().c_str (),
556+
function.get_function_name ().c_str (),
561557
trait_reference.get_name ().c_str ());
562558
}
563559
}
@@ -577,19 +573,19 @@ class TypeCheckImplItemWithTrait : public TypeCheckImplItem
577573
private:
578574
TypeCheckImplItemWithTrait (
579575
HIR::ImplBlock *parent, TyTy::BaseType *self,
580-
TraitReference &trait_reference,
576+
TyTy::TypeBoundPredicate &trait_reference,
581577
std::vector<TyTy::SubstitutionParamMapping> substitutions)
582578
: TypeCheckImplItem (parent, self), trait_reference (trait_reference),
583-
resolved_trait_item (&TraitItemReference::error_node ()),
579+
resolved_trait_item (TyTy::TypeBoundPredicateItem::error ()),
584580
substitutions (substitutions)
585581
{
586582
rust_assert (is_trait_impl_block ());
587583
}
588584

589585
bool is_trait_impl_block () const { return !trait_reference.is_error (); }
590586

591-
TraitReference &trait_reference;
592-
TraitItemReference *resolved_trait_item;
587+
TyTy::TypeBoundPredicate &trait_reference;
588+
TyTy::TypeBoundPredicateItem resolved_trait_item;
593589
std::vector<TyTy::SubstitutionParamMapping> substitutions;
594590
};
595591

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ class TypeCheckItem : public TypeCheckBase
7878
trait_reference = TraitResolver::Resolve (*ref.get ());
7979
rust_assert (!trait_reference->is_error ());
8080

81+
// we don't error out here see: gcc/testsuite/rust/compile/traits2.rs
82+
// for example
8183
specified_bound = get_predicate_from_bound (*ref.get ());
82-
// FIXME error out maybe?
83-
// if specified_Bound == TyTy::TypeBoundPredicate::error() ?
8484
}
8585

8686
TyTy::BaseType *self = nullptr;
@@ -120,9 +120,9 @@ class TypeCheckItem : public TypeCheckBase
120120
auto trait_item_ref
121121
= TypeCheckImplItemWithTrait::Resolve (&impl_block,
122122
impl_item.get (), self,
123-
*trait_reference,
123+
specified_bound,
124124
substitutions);
125-
trait_item_refs.push_back (trait_item_ref);
125+
trait_item_refs.push_back (trait_item_ref.get_raw_item ());
126126
}
127127
}
128128

@@ -134,7 +134,7 @@ class TypeCheckItem : public TypeCheckBase
134134
// filter the missing impl_items
135135
std::vector<std::reference_wrapper<const TraitItemReference>>
136136
missing_trait_items;
137-
for (auto &trait_item_ref : trait_reference->get_trait_items ())
137+
for (const auto &trait_item_ref : trait_reference->get_trait_items ())
138138
{
139139
bool found = false;
140140
for (auto implemented_trait_item : trait_item_refs)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,8 @@ class TypeCheckContext
211211
void clear_associated_type_mapping (HirId id)
212212
{
213213
auto it = associated_type_mappings.find (id);
214-
rust_assert (it != associated_type_mappings.end ());
215-
associated_type_mappings.erase (it);
214+
if (it != associated_type_mappings.end ())
215+
associated_type_mappings.erase (it);
216216
}
217217

218218
// lookup any associated type mappings, the out parameter of mapping is

gcc/rust/typecheck/rust-substitution-mapper.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,14 @@ class SubstMapperInternal : public TyTy::TyVisitor
208208
void visit (TyTy::PlaceholderType &type) override
209209
{
210210
rust_assert (type.can_resolve ());
211-
resolved = SubstMapperInternal::Resolve (type.resolve (), mappings);
211+
if (mappings.trait_item_mode ())
212+
{
213+
resolved = type.resolve ();
214+
}
215+
else
216+
{
217+
resolved = SubstMapperInternal::Resolve (type.resolve (), mappings);
218+
}
212219
}
213220

214221
void visit (TyTy::ProjectionType &type) override

0 commit comments

Comments
 (0)