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

[5.1] [NFC] Correct assumptions about static AbstractStorageDecls #23757

Merged
merged 5 commits into from Apr 3, 2019

Conversation

Projects
None yet
1 participant
@brentdax
Copy link
Collaborator

brentdax commented Apr 3, 2019

Cherry-pick of #23724 to swift-5.1-branch. The master PR was reviewed by @slavapestov and @xedin.

This PR contains some of the refactoring needed to support static subscripts:

  • Modifies AbstractStorageDecl and its subclasses to allow the AST to express the idea of a static subscript.
  • Modifies various parts of the compiler to avoid casting from AbstractStorageDecl to VarDecl before checking staticness. These changes are mostly mechanical, but I've nontrivially refactored one place in override checking.
  • Corrects a place in the constraint solver which assumes the self parameter of a subscript is never a metatype.

It does not modify the parser to accept static modifiers on subscripts, nor does it modify the type checker to allow subscripts to be applied to metatypes, so there should not be a way to create or use a static subscript. It also tries not to add much new code which will only be exercised when handling a static subscript.

I'd like to land these changes early to give them a little more time to bake and to avoid repeated merge conflicts, particularly on the serialization version number. This PR by itself should not cause any behavior changes; #23358, which implements static subscripts, includes tests verifying that the relevant features behave correctly with static subscripts.

@brentdax

This comment has been minimized.

Copy link
Collaborator Author

brentdax commented Apr 3, 2019

@swift-ci please test

brentdax added some commits Mar 16, 2019

[NFC] AST-level support for static subscripts
* Moves the IsStatic flag from VarDecl to AbstractStorageDecl.
* Adds a StaticSubscriptKind to SubscriptDecl.
* Updates serialization for these changes.
* Updates SubscriptDecl constructor call sites for these changes.

# Conflicts:
#	include/swift/Serialization/ModuleFormat.h
[NFC] Remove VarDecl guards on staticness checks
Fixes various places where we assume that only a VarDecl can be static so they operate on any AbstractStorageDecl instead. NFC until static subscripts are added.
[NFC] Correct constraint system assumption
We currently assume that, if a subscript is declared within a value type’s decl, it must need `self` to be passed inout. This isn’t true for static subscripts, because even though the DeclContext is a value type, the metatype is actually a reference type. Skip this check for non-instance members.

NFC until static subscripts are added.
[NFC] Override-check decls by property, not kind
Rather than figuring out which kind of decl we’re using and then testing the traits it has, check each trait once on the decls where the AST can express it.

This change is NFC currently, but will support static subscripts.

@brentdax brentdax force-pushed the brentdax:static-start-5.1 branch from cb60d89 to d9bd7a7 Apr 3, 2019

@brentdax

This comment has been minimized.

Copy link
Collaborator Author

brentdax commented Apr 3, 2019

@swift-ci please test

@apple apple deleted a comment from swift-ci Apr 3, 2019

@apple apple deleted a comment from swift-ci Apr 3, 2019

@brentdax brentdax requested a review from slavapestov Apr 3, 2019

@brentdax brentdax merged commit cefa256 into apple:swift-5.1-branch Apr 3, 2019

4 checks passed

Swift Test Linux Platform No test results found.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform No test results found.
Details
Swift Test OS X Platform (smoke test)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.