[Type checker] Use DependentMemberType instead of type variables for nested types. #5626
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please smoke test and merge |
|
Darn. Fndation/NSScanner.swift:605:49: error: 'Int1' is not convertible to 'Bool' |
|
Reduced bug: |
Bugs aside, this is awesome! |
slavapestov
reviewed
Nov 3, 2016
| return type.transform([&](Type type) -> Type { | ||
| if (auto tvt = dyn_cast<TypeVariableType>(type.getPointer())) { | ||
| auto known = typeBindings.find(tvt); | ||
| assert(known != typeBindings.end()); | ||
| return known->second; | ||
| } | ||
|
|
||
| // If this is a dependent member type for which we end up simplifying |
| auto dictionaryValueTy = CS.getMemberType(dictionaryTy, | ||
| valueAssocTy, | ||
| locatorBuilder.withPathElement( | ||
| ConstraintLocator::ArrayElementType), |
There was a problem hiding this comment.
is ArrayElementType not used anymore?
There was a problem hiding this comment.
Sadly, it's still used... actually, abused, because it's used for completely unrelated things.
The constraint solver scales poorly with the number of type variables, so track it.
…nested types. In the constraint solver, we've traditionally modeled nested type via a "type member" constraint of the form $T1 = $T0.NameOfTypeMember and treated $T1 as a type variable. While the solver did generally try to avoid attempting bindings for $T1 (it would wait until $T0 was bound, which solves the constraint), on occasion we would get weird behavior because the solver did try to bind the type variable. With this commit, model nested types via DependentMemberType, the same way we handle (e.g.) the nested type of a generic type parameter. This solution maintains more information (e.g., we know specifically which associated type we're referring to), fits in better with the type system (we know how to deal with dependent members throughout the type checker, AST, and so on), and is easier to reason able. This change is a performance optimization for the type checker for a few reasons. First, it reduces the number of type variables we need to deal with significantly (we create half as many type variables while type checking the standard library), and the solver scales poorly with the number of type variables because it visits all of the as-yet-unbound type variables at each solving step. Second, it eliminates a number of redundant by-name lookups in cases where we already know which associated type we want. Overall, this change provides a 25% speedup when type-checking the standard library.
The ASTContext had a wacky "get member type" callback that actually called back into the constraint system (!) to build member types. This callback was obsoleted by the change that started representing nested types as DependentMemberTypes.
The constraint solver tries not to solve for type variables that "involve other type variables", which handles the case where we have seen a constraint that mentions the type variable under consideration as well as a different type variable, but in a constraint that we cannot capture in a binding. Solving for such type variables too early can lead to missed solutions, so we avoid it. Tweak the logic for this computation to not consider type variables mentioned within dependent member types (e.g., $T0.Iterator.Element), because such types do not affect type inference at all, and therefore shouldn't prevent solving for the type variable in question.
DougGregor
force-pushed
the
dependent-type-variables
branch
from
6b1a8c1
to
ba06709
Compare
|
@swift-ci please smoke test and merge |
The code in TypeBase::gatherAllSubstitutions() walks a declaration context and type to form a complete set of substitutions for a (nested) type. When provided with a context for a nested extension---e.g., "extension OuterGeneric.InnerGeneric", it would skip over OuterGeneric due to the lexical nesting of DeclContexts not matching the semantic nesting. Teach this function to determine the type parameters without walking DeclContexts.
|
... fixed a latent bug in |
|
@swift-ci please smoke test and merge |
1 similar comment
|
@swift-ci please smoke test and merge |
|
@swift-ci please smoke test Linux |
1 similar comment
|
@swift-ci please smoke test Linux |
|
@DougGregor Thank you! Excellent stuff! :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the constraint solver, we've traditionally modeled nested type via
a "type member" constraint of the form
and treated $T1 as a type variable. While the solver did generally try
to avoid attempting bindings for $T1 (it would wait until $T0 was
bound, which solves the constraint), on occasion we would get weird
behavior because the solver did try to bind the type
variable.
With this commit, model nested types via DependentMemberType, the same
way we handle (e.g.) the nested type of a generic type parameter. This
solution maintains more information (e.g., we know specifically which
associated type we're referring to), fits in better with the type
system (we know how to deal with dependent members throughout the type
checker, AST, and so on), and is easier to reason able.
This change is a performance optimization for the type checker for a
few reasons. First, it reduces the number of type variables we need to
deal with significantly (we create half as many type variables while
type checking the standard library), and the solver scales poorly with
the number of type variables because it visits all of the
as-yet-unbound type variables at each solving step. Second, it
eliminates a number of redundant by-name lookups in cases where we
already know which associated type we want.
Overall, this change provides a 25% speedup when type-checking the
standard library.