[SE-0284] Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers #29735

owenv · 2020-02-10T04:13:52Z

The restriction that the argument following a variadic argument must have a label is still in place. This allows the following:

func twoVariadics(_ a: Int..., b: Int...) { }
func splitUnlabeledVariadics(_ a: Int..., b: Int, _ c: String...) { }

Because varargs get automatic default expressions of [], the following is also allowed right now, and maybe shouldn't be:

func splitByDefaultArgVariadics(_ a: Int..., b: Int = 0, _ c: Int...) { }
splitByDefaultArgVariadics(1,2,3, b: 4, 5, 6, 7) // a is [1,2,3], b is 4, c is [5,6,7]
splitByDefaultArgVariadics(1,2,3) // a is [1,2,3], b is 0, c is []

The alternative is to just require every vararg have a label if there's more than one, which won't be difficult to enforce if that ends up being preferable.

owenv · 2020-02-10T04:14:25Z

Oops, I meant to submit this as a draft

theblixguy · 2020-02-10T09:35:41Z

Nice. Does this SILGen and run properly as well?

owenv · 2020-02-10T17:01:24Z

@theblixguy yep, although I need to write some tests for it at some point :). Everything just gets passed around in Arrays so having 2+ hasn’t been a problem.

owenv · 2020-02-12T04:58:20Z

@swift-ci please test

owenv · 2020-02-12T04:58:33Z

@swift-ci please test source compatibility

owenv · 2020-02-12T18:12:13Z

That’s odd, I’m seeing some unexpected passes on the source compatibility suite...

owenv · 2020-02-15T22:39:43Z

Let's see if we might be able to get away with making the following declarations an error:

subscript(a: Int..., b: Int)
let closure = {(a: Int..., b: Int) in}

These are allowed today because of a bug, but there's no way to successfully call them. It's also possible to write:

subscript(a: Int..., b: Int = 1)

which can be called (but b can't be manually specified), but really shouldn't be allowed.

If the source break is too large, I'll probably try to make these at least a warning as part of the proposal.

owenv · 2020-02-15T22:39:50Z

@swift-ci please test

owenv · 2020-02-15T22:40:02Z

@swift-ci please test source compatibility

owenv · 2020-02-16T02:17:34Z

Looks like swift-nio and swift-nio-extras are the only failures and are affecting other PRs as well.

owenv · 2020-03-25T19:52:54Z

@swift-ci please test

owenv · 2020-03-25T19:53:17Z

@swift-ci please test source compatibility

swift-ci · 2020-03-25T21:07:36Z

Build failed
Swift Test Linux Platform
Git Sha - 42c8912afa199741f6566f90bff9e0c5eae6b02c

owenv · 2020-03-25T22:59:54Z

@swift-ci please test Linux

owenv · 2020-05-30T23:46:37Z

rebasing on recent changes

@swift-ci please test

owenv · 2020-05-30T23:46:54Z

@swift-ci please test source compatibility

swift-ci · 2020-05-31T00:57:58Z

Build failed
Swift Test Linux Platform
Git Sha - a83f19b7f0f5ee61325646ce5b383ac8fc649ce2

owenv · 2020-07-19T21:21:42Z

@swift-ci test

owenv · 2020-07-19T21:21:55Z

@swift-ci test source compatibility

owenv · 2020-07-19T23:42:54Z

@swift-ci please test Linux

rintaro

Parser changes looks great 👍
Could you add some test cases in test/Syntax/round_trip_parse_gen.swift?

rintaro · 2020-07-20T16:37:54Z

test/decl/func/vararg.swift

+let closure2 = {(x: Int..., y: Int) in } // expected-error {{no parameters may follow a variadic parameter in a closure}}
+let closure3 = {(x: Int..., y: Int, z: Int...) in } // expected-error {{no parameters may follow a variadic parameter in a closure}}
+let closure4 = {(x: Int...) in }
+let closure5 = {(x: Int, y: Int...) in }


I guess {(x: Int..., y z: Int) in } is still diagnosed as error: closure cannot have keyword arguments, but it should be error: no parameters may follow a variadic parameter in a closure because removing the argument label doesn't resolve the issue.

It would be great if we can improve this. (not in this PR)

I added a test case for this, right now both diagnostics are emitted.

test/decl/func/vararg.swift

owenv · 2020-07-21T06:58:44Z

@swift-ci smoke test

rintaro · 2020-07-21T17:02:36Z

@swift-ci Please smoke test macOS

rintaro

Parser change looks good.

rintaro · 2020-07-21T22:57:05Z

@swift-ci Please smoke test macOS

owenv · 2020-07-24T04:49:42Z

@swift-ci please test Windows platform

[SE-0284] Add round_trip_parse_gen tests [SE-0284] Add missing test cases

owenv · 2020-08-24T04:37:59Z

@swift-ci test

owenv · 2020-08-24T04:41:06Z

I added a couple more argument matching tests for forward matching of multiple trailing closures + multiple varargs now that SE-286 is implemented.

@xedin do you mind reviewing the argument matching test cases before this is merged? There aren't any code changes to argument matching, but I want to make sure I haven't missed any newly introduced edge cases

xedin · 2020-08-24T19:27:12Z

@owenv Sure, I'll take a look as soon as possible!

xedin

Tests look reasonable to me!

owenv · 2020-09-04T04:16:35Z

Thanks for all the help on this @rintaro and @xedin!

owenv · 2020-09-04T04:17:25Z

Rerunning tests before I merge this since the master-rebranch merge happened recently

@swift-ci test

owenv · 2020-09-04T04:17:38Z

@swift-ci please test source compatibility

swift-ci · 2020-09-04T06:17:28Z

Build failed
Swift Test Linux Platform
Git Sha - 9708247

owenv · 2020-09-04T14:47:01Z

@swift-ci please test Linux platform

owenv · 2020-09-05T16:27:09Z

Looks like the debug source compatibility failures (BlueSocket and ReactiveCocoa) are also happening in all other PRs from the past few days, so unrelated to this change.

rintaro · 2020-09-05T17:37:12Z

@owenv Thank you! Could you update CHANGELOG.md as well?

owenv changed the title ~~[DNM] Experiment: Lift the 1-vararg-per-function restriction~~ [DNM, Draft] Experiment: Lift the 1-vararg-per-function restriction Feb 10, 2020

owenv added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Feb 10, 2020

owenv force-pushed the multiple-varargs branch from 068ded7 to baa4333 Compare February 11, 2020 00:30

owenv changed the title ~~[DNM, Draft] Experiment: Lift the 1-vararg-per-function restriction~~ [Pending-Evolution] Lift the 1-vararg-per-function restriction Feb 11, 2020

owenv force-pushed the multiple-varargs branch from baa4333 to 5d30379 Compare February 11, 2020 16:30

fassko mentioned this pull request Feb 14, 2020

[154] Issue #154 - February 27, 2020 SwiftWeekly/swiftweekly.github.io#500

Closed

owenv force-pushed the multiple-varargs branch from 5d30379 to fe2c00c Compare February 15, 2020 22:34

This comment has been minimized.

Sign in to view

owenv mentioned this pull request Feb 19, 2020

Add proposal: Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers apple/swift-evolution#1125

Merged

owenv force-pushed the multiple-varargs branch from fe2c00c to 42c8912 Compare March 25, 2020 19:52

owenv force-pushed the multiple-varargs branch 2 times, most recently from c0d931a to a83f19b Compare May 30, 2020 23:46

apple deleted a comment from swift-ci Jul 19, 2020

owenv requested review from CodaFi and rintaro July 20, 2020 02:32

rintaro reviewed Jul 20, 2020

View reviewed changes

test/decl/func/vararg.swift Show resolved Hide resolved

owenv force-pushed the multiple-varargs branch from fccafc0 to c8382ec Compare July 21, 2020 06:57

rintaro approved these changes Jul 21, 2020

View reviewed changes

[SE-0284] Lift the 1-vararg-per-function restriction

9708247

[SE-0284] Add round_trip_parse_gen tests [SE-0284] Add missing test cases

owenv force-pushed the multiple-varargs branch from c8382ec to 9708247 Compare August 24, 2020 04:37

owenv requested a review from xedin August 24, 2020 04:41

xedin approved these changes Sep 3, 2020

View reviewed changes

owenv merged commit c6a266e into apple:master Sep 5, 2020

owenv mentioned this pull request Sep 5, 2020

[Changelog] Add SE-0284 #33828

Merged

[SE-0284] Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers #29735

[SE-0284] Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers #29735

Conversation

owenv commented Feb 10, 2020

owenv commented Feb 10, 2020

theblixguy commented Feb 10, 2020

owenv commented Feb 10, 2020

owenv commented Feb 12, 2020

owenv commented Feb 12, 2020

owenv commented Feb 12, 2020

owenv commented Feb 15, 2020

owenv commented Feb 15, 2020

owenv commented Feb 15, 2020

This comment has been minimized.

This comment has been minimized.

owenv commented Feb 16, 2020

owenv commented Mar 25, 2020

owenv commented Mar 25, 2020

swift-ci commented Mar 25, 2020

owenv commented Mar 25, 2020

owenv commented May 30, 2020

owenv commented May 30, 2020

swift-ci commented May 31, 2020

owenv commented Jul 19, 2020

owenv commented Jul 19, 2020

owenv commented Jul 19, 2020

rintaro left a comment

Choose a reason for hiding this comment

rintaro Jul 20, 2020

Choose a reason for hiding this comment

owenv Jul 21, 2020

Choose a reason for hiding this comment

owenv commented Jul 21, 2020

rintaro commented Jul 21, 2020

rintaro left a comment

Choose a reason for hiding this comment

rintaro commented Jul 21, 2020

owenv commented Jul 24, 2020

owenv commented Aug 24, 2020

owenv commented Aug 24, 2020

xedin commented Aug 24, 2020

xedin left a comment

Choose a reason for hiding this comment

owenv commented Sep 4, 2020

owenv commented Sep 4, 2020

owenv commented Sep 4, 2020

swift-ci commented Sep 4, 2020

owenv commented Sep 4, 2020

owenv commented Sep 5, 2020

rintaro commented Sep 5, 2020