Skip to content

Drop witness markers for 'uninteresting' dependent types #5687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 9, 2016

Conversation

DougGregor
Copy link
Member

This pull request removes witness markers for 'uninteresting' dependent types--e.g, those dependent types for which there are no additional protocol conformances required beyond what is implied by other constraints--from substitution lists and the list of "all" dependent types in a generic signature. For example, given something like:

protocol P { }
protocol Q {
  associatedtype Assoc : P
}

a generic signature <T: Q> currently would contain witness markers for both T and T.Assoc, even though everything about T.Assoc is recoverable from the conformance of T to Q. With this change, we would only record a witness marker for T.

It's a necessary step toward implementing recursive protocol conformances (because the list of "all" witness markers would be infinite were Assoc to conform to Q, i.e., T.Assoc, T.Assoc.Assoc, T.Assoc.Assoc.Assoc, etc.). It's also a cleaner way to model the problem, and a step toward eliminating witness markers altogether.

This pull request is still a work-in-progress for two reasons:

  • It has an unexpected impact on associated type inference, which hit the standard library in two places that caused breaking. One involves the inferred Indices witness for a minimal RandomAccessCollection model, and the other involves a change in inference for the SubSequence of the existential collections. The former is potentially a bug fix to the type checker that may imply a library change, while the latter appears to be an outright bug in the inference of associated type witnesses.
  • It breaks property behaviors, which depend on some dodgy uses of protocols that break when we're forced to use an actual protocol conformance to resolve associated type witnesses.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

Alright, I clearly need to address this associated type witness inference issue before I have any hope of landing this change.

slavapestov and others added 8 commits November 8, 2016 16:11
Recently I changed the ArchetypeBuilder is minimize requirements
in generic signatures. However substitution lists still contained
all recursively-expanded nested types.

With recursive conformances, this list becomes potentially
infinite, so we can't expand it out anymore. Also, it is just
a waste of time to have them there.
Extensions of the RandomAccessCollection protocol provide two
mutually-incompatible default implementations for 'indices', one of
which uses DefaultRandomAccessIndices<Self> and the other uses
CountableRange<Index>. Type witness inference (rightly) considers the
two ambiguous, although it didn't before, causing regressions.

Remove the same-type constraint

    SubSequence == DefaultRandomAccessIndices<Self>

from the most general extension, so that the more-specialized,
CountableRange version is selected when it's constraints are met, and
the most-general version is selected otherwise.

The down-side to this change is that the most-general version of
'indices', which produces a DefaultRandomAccessIndices<Self>, will be
available in all types that conform to RandomAccessCollection, even if
they have a different Indices type.
…llection.

The BidirectionalCollection protocol has no requirements that mention
the associated type 'SubSequence', which means it cannot be easily
inferred. Duplicate the subscript requirement from Collection into
BidirectionalCollection to aid type witness inference.
…e" check.

Expand the context conversion failure check used to fix
rdar://28909024 to cover the case where we have a single-argument
mismatch and there is a type parameter deduction as well.
They're relying on having extraneous witness markers to work around
some oddities in the modeling of property behaviors as protocols.
Debugging type witness inference is annoying; add some dumping code to
help with debugging.
@DougGregor DougGregor force-pushed the drop-witness-markers branch from 66b665d to eacf223 Compare November 9, 2016 00:11
@DougGregor DougGregor changed the title [WIP] Drop witness markers for 'uninteresting' dependent types Drop witness markers for 'uninteresting' dependent types Nov 9, 2016
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

…lection.

Similar to what we did for BidirectionalCollection, duplicate the
subscript-returning-SubSequence requirement in
RandomAccessCollection.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
Copy link
Member Author

I don't think that timeout is related to my changes

@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
Copy link
Member Author

@swift-ci please merge

@DougGregor DougGregor merged commit e2f8d4d into swiftlang:master Nov 9, 2016
@DougGregor DougGregor deleted the drop-witness-markers branch November 9, 2016 02:43
@DougGregor
Copy link
Member Author

This sped up type checking of the standard library by 11%. Cool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants