Various cleanups for GenericEnvironment
#5966
Conversation
The uses of this function that want *all* nested types now go through an entry point getAllNestedTypes(), and will need to be removed to support recursive protocol constraints. The uses of this function that only want to see what's been expanded so far---dumpers and verifiers, mainly---can use getKnownNestedTypes(), which may change type but is a reasonable operation to continue using.
ASTContext-allocated objects don't get destructed unless they are registered; make sure to do that for GenericEnvironments, because we're leaking them like crazy.
GenericEnvironment walked its input mapping in DenseMap order while populating a reverse mapping from archetypes to interface types. However, this mapping is not unique, because two generic parameters can end up mapping to the same archetype. In such cases, which generic parameter we mapped to was nondeterministic. Make this deterministic by preferring to map back to the earlier generic parameter.
|
@swift-ci please smoke test and merge |
| @@ -53,6 +53,9 @@ GenericEnvironment::GenericEnvironment( | |||
| // FIXME: If multiple generic parameters map to the same archetype, | |||
| // the reverse mapping order is not deterministic. | |||
| } | |||
|
|
|||
| // Make sure this generic environment gets destroyed. | |||
| signature->getASTContext().addDestructorCleanup(*this); | |||
There was a problem hiding this comment.
This makes me unhappy; it seems like it could greatly increase the number of destructor cleanups we need. Since the mapping won't change during the lifetime of the generic environment, can we use some kind of sorted list and tail-allocate the storage instead?
There was a problem hiding this comment.
Eventually, yes. The problem is that Type::subst(), which is used a lot with these mappings in GenericEnvironment, requires aTypeSubstitutionMap. So we'd be unpacking the mapping a whole lot of times.
A better approach would be to teach Type::subst() and related not to use a DenseMap as their way of providing mappings. That'll take a bit more work, but is the right thing to do long-term.
Two cleanups for
GenericEnvironment:Plug a memory leak:
ASTContext-allocated types don't get destroyed unless registered explicitly, so we were leaking theTypeSubstitutionMapmembers ofGenericEnvironment.Eliminate nondeterministic behavior in the mapping from archetypes to interface types when two generic parameters map to the same archetype.