Skip to content

Default arguments: implement captures for local functions, diagnose 'Self' for methods #27220

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 6 commits into from
Sep 18, 2019
Merged

Default arguments: implement captures for local functions, diagnose 'Self' for methods #27220

merged 6 commits into from
Sep 18, 2019

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Sep 17, 2019 •
edited
Loading

  • Diagnose 'Self' in class method default arguments, since it's not supported
  • Implement local function default argument captures
  • Fix two other crashes found by inspection

I'll add a changelog entry in a new PR since I've already started tests on this one. This fixes stuff like

func outer(x: Int) {
  func inner(y: Int = x) {}

  inner() // equivalent to inner(y: x)
}

Fixes https://bugs.swift.org/browse/SR-2189, rdar://problem/20796451, rdar://problem/55119566.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

jrose-apple
jrose-apple reviewed Sep 18, 2019
View reviewed changes
include/swift/AST/Decl.h
@@ -5164,6 +5164,7 @@ class ParamDecl : public VarDecl {
PointerUnion<Expr *, VarDecl *> DefaultArg;
Initializer *InitContext = nullptr;
StringRef StringRepresentation;
CaptureInfo Captures;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, uh, kind of expensive to put in every default argument, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, but I don't think there are that many default arguments. We can shrink it down to the size of a single ArrayRef with a bit of work though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you'll have successfully nerd-sniped me into it after all!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, you're more than welcome to. I have another commit I'm going to tack onto this PR that implements default argument captures for local functions -- it wasn't too hard after all. Might want to wait for that one to avoid too many merge conflicts

@slavapestov
Sema: Compute and store captures for default argument expressions
67c6682
@slavapestov
SIL: Refactor type lowering computation of the effective generic sign…
e1e6f0e 
…ature

This also fixes a long-standing bug with default argument expressions
capturing generic parameters.
@slavapestov
Sema: Diagnose captures of dynamic 'Self' type from default argument …
d3a4f3d 
…expressions

We could actually allow this for local functions, but it's not
worth implementing that until the more general ability of local
function default arguments to capture values is implemented.

Fixes <rdar://problem/55119566>.
@slavapestov
SILGen: Fix crash when immediately-applied closure transitively captu…
b8642b0 
…res generic parameters
@slavapestov
SIL: Compute lowered local captures of SILDeclRefs and not AnyFunctio…
533d41d 
…nRefs

Unfortuantely this commit is bigger than I would like but I couldn't think
of any reasonable ways to split it up.

The general idea here is that capture computation is now done for a
SILDeclRef and not an AnyFunctionRef. This allows SIL to represent the
captures of a default argument generator.
@slavapestov
SILGen: Support for local function default argument captures
78ae7ac 
Fixes <https://bugs.swift.org/browse/SR-2189>, <rdar://problem/20796451>.
@slavapestov slavapestov force-pushed the default-arg-self-capture-diagnostic branch from 9534b07 to 78ae7ac Compare September 18, 2019 18:26
@slavapestov slavapestov changed the title Fix crash when default argument expression captures dynamic 'Self' Default arguments: implement captures for local functions, diagnose 'Self' for methods Sep 18, 2019
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov merged commit 4fcef2b into swiftlang:master Sep 18, 2019
slavapestov added a commit to slavapestov/swift that referenced this pull request Oct 22, 2019
@slavapestov
Changelog entry for swiftlang#27220
317e97c
slavapestov added a commit that referenced this pull request Oct 22, 2019
@slavapestov
Merge pull request #27828 from slavapestov/changelog-entry-local-defa…
9d047d9 
…ult-arg-captures

Changelog entry for #27220
@slavapestov slavapestov mentioned this pull request Sep 19, 2019
Closed
@AnthonyLatsis AnthonyLatsis mentioned this pull request Jan 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple jrose-apple left review comments

@DougGregor DougGregor DougGregor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@slavapestov @DougGregor @jrose-apple