Skip to content

[Name lookup] Introduce a request for "extended nominal type decl" #18425

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 6, 2018
Merged

[Name lookup] Introduce a request for "extended nominal type decl" #18425

merged 5 commits into from
Aug 6, 2018

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Jul 31, 2018 •
edited
Loading

Introduce ExtensionDecl::getExtendedNominal() to provide the nominal
type declaration that the extension declaration extends. Move most
of the existing callers of the callers of getExtendedType() over to
getExtendedNominal(), because they don’t need the full type information.

Introduce a request to back ExtensionDecl::getExtendedNominal(), which
uses the type-decl-resolution path that doesn't go through a Type. Use that
to reduce the dependence of the early "extension binding" pass (which associates
extension declarations with nominal types) on the type checker.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor DougGregor changed the title [AST] Add ExtensionDecl::getExtendedNominal(). [Name lookup] Introduce a request for "extended nominal type decl" Aug 2, 2018
@slavapestov
Copy link
Contributor

The line notes are messed up again, gah!

Basically I’m confused why we ever call bindExtension() directly - are there invalid nested extensions that do lookup into other extensions during binding or something?

@DougGregor
Copy link
Member Author

(Writing here because my rebase made line-specific commentary hard to follow)

@slavapestov I confused the issue because I effectively added another level of "binding" for extensions without giving it a name. Here are the three effective "levels":

  • Determining the nominal type declaration to which the extension is bound, which makes the extension visible to getExtensions(). This is what we do in the worklist-based early type checking pass.
  • Determining the type to which the extension is bound, as an actual Type, which lets us talk about the Self type in context. The calls through LazyResolver::bindExtension() do this.
  • Computing a full generic signature for the extension. The calls through LazyResolver::resolveExtension(). do this.

Prior to this pull request, the first two levels were the same. This pull request splits them apart, creates a request for the former, and scatters bindExtension() calls around where clients want the type information from the second level. Request-ifying the second level will eliminate the bindExtension() calls, but for now I'd like them to be there because it's a good forcing function for moving these clients over to getExtendedNominal(), which is cheaper and doesn't require the type checker.

@DougGregor
Copy link
Member Author

#18490 removed the getAllConformances() call that broke the Linux build, so now I'll try that again.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
[AST] Add ExtensionDecl::getExtendedNominal().
bd5f5d8 
Introduce ExtensionDecl::getExtendedNominal() to provide the nominal
type declaration that the extension declaration extends. Move most
of the existing callers of the callers to getExtendedType() over to
getExtendedNominal(), because they don’t need the full type information.

ExtensionDecl::getExtendedNominal() is itself not very interesting yet,
because it depends on getExtendedType().
@DougGregor
[Name lookup] Introduce a request for “extended nominal type decl”.
84cb197 
Introduce a request for ExtensionDecl::getExtendedNominal() that
uses TypeRepr-based resolution to find the extended nominal
type declaration without going through type resolution.
@DougGregor
[AST] Replace TypeBase::getGenericAncestor() with ClassDecl::getGener…
8dd40bb 
…icAncestor().

This operation does not require type information, so move it over to
ClassDecl where this information is more generally useful.
@DougGregor
[Type checker] Eliminate type checking from early extension binding.
3018bec 
Use ExtensionDecl::getExtendedNominal() to wire up extensions to their
nominal types early in type checking (the bindExtensions()) operation,
rather than going through type validation to do so.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
[Type checker] Get all conformances for externally-defined types.
bc630b7 
IR generation depends on these when it emits type metadata.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test Linux

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

@DougGregor
Copy link
Member Author

@swift-ci please test Linux

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test Linux

@DougGregor DougGregor merged commit 199bf6e into swiftlang:master Aug 6, 2018
@DougGregor DougGregor deleted the extended-nominal branch August 6, 2018 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DougGregor @slavapestov