Wean AbstractFunctionDecl constructors off multiple parameter lists #18128
Conversation
This file contains hidden or 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
There was some dead code that's no longer needed.
…meter lists There are two general constructor forms here: - One took the number of parameter lists, to be filled in later. Now, this takes a boolean indicating if there is an implicit 'self'. - The other one took the actual parameter lists and filled them in right away. This now takes a separate 'self' ParamDecl and ParameterList. Instead of storing the number of parameter lists, an AbstractFunctionDecl now only needs to store if there is a 'self' or not. I've updated most places that construct AbstractFunctionDecls to properly use these new forms. In the ClangImporter, there is more code that remains to be untangled, so we continue to build multiple ParameterLists and unpack them into a ParamDecl and ParameterList at the last minute.
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
jrose-apple
reviewed
Jul 23, 2018
| @@ -490,7 +490,7 @@ static void handleSILDeclRef(Serializer &S, const SILDeclRef &Ref, | |||
| SmallVectorImpl<ValueID> &ListOfValues) { | |||
| ListOfValues.push_back(S.addDeclRef(Ref.getDecl())); | |||
| ListOfValues.push_back((unsigned)Ref.kind); | |||
| ListOfValues.push_back(Ref.getParameterListCount() - 1); | |||
| ListOfValues.push_back(Ref.isCurried); | |||
There was a problem hiding this comment.
This is a format change; please bump the module version. Doing it in a separate commit confused me. You're good.
jrose-apple
reviewed
Jul 23, 2018
| @@ -5341,19 +5340,18 @@ class FuncDecl : public AbstractFunctionDecl { | |||
| SourceLoc FuncLoc, | |||
| DeclName Name, SourceLoc NameLoc, | |||
| bool Throws, SourceLoc ThrowsLoc, | |||
| unsigned NumParameterLists, | |||
| bool HasImplicitSelfDecl, | |||
There was a problem hiding this comment.
Is this boolean ever distinct from Parent->isTypeContext()?
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.
This is a follow-up to #18082 and #18102. Now that most usages of
getParameterLists(),getNumParameterLists(), etc are gone, the next step is to change how function declarations are constructed. Instead of taking an array of parameter lists, they now take a self declaration and single parameter list. The self declaration is required forConstructorDeclandDestructorDecl; forFuncDecl, it is required iff the function is nested inside of a nominal type.