Skip to content
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

[Type checker] Introduce requests for @objc, override, and 'dynamic' checking #17976

Merged
merged 32 commits into from Jul 19, 2018

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Jul 16, 2018

Introduce new type checker requests to cover three more aspects of type-checking the interface of a declaration, peeled off from validateDecl and mostly separated from the type checker:

  • ValueDecl::getOverriddenDecl(s): determines which declaration(s) are overridden by a given declaration
  • ValueDecl::isObjC(): determines whether the given declaration is exposed to Objective-C.
  • ValueDecl::isDynamic(): a replacement for directly querying DynamicAttr on a declaration, this determines whether a particular declaration is 'dynamic'.

All of these effectively need to be done together to break down otherwise-cyclic dependencies, e.g., because @objc can be inherited from an override and dynamic is dependent on @objc (as well as being inherited).

@DougGregor
Copy link
Member Author

The computation of 'dynamic' was also tangled up with @objc and overrides, so separate that into its own request.

@DougGregor
Copy link
Member Author

Rebased. Here goes nothing!

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test compiler performance

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test compiler performance

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 29b15ad0de4e8510dd15ca8b289dbeeae984eb05

@swift-ci
Copy link
Collaborator

Build comment file:

Compilation-performance test failed

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 29b15ad0de4e8510dd15ca8b289dbeeae984eb05

@DougGregor
Copy link
Member Author

Well, that went poorly.

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 936a43a0f8aefda9ff409c92d32af3d15b942436

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 936a43a0f8aefda9ff409c92d32af3d15b942436

@hamishknight
Copy link
Collaborator

Nice! I believe this will also fix SR-5317 :)

The use of getAsClassOrClassExtensionContext() obviates the need for a
specific isTypeContext() check. NFC.
…ookup.

Make the TypeChecker’s name-lookup routines static, so they don’t depend
on the TypeCheck instance… except for one pesky place where we jump back
into protocol conformance checking. This is part of teasing apart the type
checker.
…nyRequest

The default move constructor and move assignment operator of AnyRequest
would leave the source object in an odd state that’s destructible but
does not maintain the invariant that all “normal” states store a real
instance. This state breaks DenseMap, which assumes that a moved-from
object is still washable.
Don’t ask to compute overridden declarations that haven’t already been
computed.
Introduce a new request kind to capture the computation of the set of
overridden declarations of a given declaration, eliminating the
stateful “setOverriddenDecls()” calls from the type checker.
The overridden declarations of accessors are computed; they don’t need to
be set explicitly.
The GenericSignatureBuilder is generating a *huge* number of invocations
of AssociatedTypeDecl::getOverriddenDecls(), causing contention in
the request-evaluator’s core data structures and a significant
slow-down in compile-time performance when the request-evaluator is
handling this computation. 

Those data structures need to be optimized, but in the meantime, 
introduce a performance hack to use the cached entry without going
through the request-evaluator. This will MISS dependencies and needs
to go away quickly.
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test compiler performance

@swift-ci
Copy link
Collaborator

!!! Couldn't read commit file !!!

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 0966cd9

@DougGregor
Copy link
Member Author

Thanks @hamishknight , I'll try the example from that bug!

TinyPtrVector is a more-space-efficient SmallVector<_, 1>. Use it.
When Objective-C interop was enabled, we were getting overrides
precomputed as part of the isObjC() check. Explicitly make sure we
get overrides precompiled for non-Objective-C-based platforms.
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - a733aed

@DougGregor DougGregor changed the title [Type checker] Introduce requests for @objc and override checking [Type checker] Introduce requests for @objc, override, and 'dynamic' checking Jul 19, 2018
A bunch of tests that technically require Objective-C interop were
not labeled as such, and worked in the absence of Objective-C interop
due to bugs in the type checker.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
Copy link
Member Author

@swift-ci please test Linux

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test Linux

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.

None yet

3 participants