SE-0284: Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers

The review of SE-0284: Allow Multiple Variadic Parameters in Functions, Subscripts, and Initializers begins now and runs through July 16, 2020.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via email or direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Saleem Abdulrasool
Review Manager

20 Likes

How would this syntax interact with the new multi closure syntax and how would it impact the proposed parsing change?

1 Like

I’m broadly in support of this proposal, and think the "variadic parameters must be followed by a labeled parameter" is an intuitive rule. I also agree that it's reasonable to treat the minor source break that the proposal identifies as a bug fix rather than a true source compatibility issue. Like @Jon_Shier, I hope that @owenv or @Douglas_Gregor could comment on how/whether this proposal would affect future evolution for trailing closures—we have a lot of syntactic changes going in around argument lists so we should approach them with full consideration of the mutual consequences!

This proposal looks great to me.

They're mostly orthogonal to each other, going from 1 variadic parameter to 2+ doesn't really affect how each individual variadic parameter interacts with the rest of the parameter list.

There was a brief discussion of this in the pitch thread:

I haven't thoroughly reviewed Doug's implementation of the proposed forward scanning rule yet, but I think it's very unlikely this change will interfere with it.

1 Like

This is a straight forward change to Swift, and I'm excited to see it in!

looks good

big +1

// Before
try assertArgs("swift", "-foo", "-bar", parseTo: .interactive, leaving: ["-foo", "-bar"])
// After
try assertArgs("swift", "-foo", "-bar", parseTo: .interactive, leaving: "-foo", "-bar")

This results in a cleaner, more consistent interface.

I'm puzzled by the example and the conclusion here. IMO the Before case is a clear win in terms of understanding when I'm looking at the call-site. The After case makes it look like "-bar" is a separate unlabeled argument. The [ ] delimiters in the Before case make it clear that "-bar" is part of the argument labeled leaving:.

If I were a project maintainer, I would flag this during code review as a negative. If eliding a couple of brackets leads to ambiguity when reading, might as well write those two brackets. One can use array arguments in both places if one strongly desires consistency.


myView.addSubviews(v1, v2, constraints: v1.widthAnchor.constraint(equalTo: v2.widthAnchor),
                                        v1.heightAnchor.constraint(equalToConstant: 40),
                                        /* More Constraints... */)

This example seems hand-formatted... how is a code formatter supposed to format it this way? With long lines, what is most likely going to happen is something like:

myView.addSubviews(
    v1,
    v2,
    constraints: myVeryVeryVeryVeryLooooooooooooooooongConstraint,
    myOtherVeryVeryLooooooooooongConstraintExpression
)

:slightly_frowning_face: Again the same problem comes up. While reading, someone is likely to misunderstand what is going on. Making the formatting like the hand-formatted example requires a code formatter to do non-trivial semantic analysis. Does swift-format already do this?

Having an array sidesteps the issue:

myView.addSubviews(
    v1,
    v2,
    constraints: [
        myVeryVeryVeryVeryLooooooooooooooooongConstraint,
        myOtherVeryVeryLooooooooooongConstraintExpression
    ]
)

When I started reading the proposal, I was ambivalent about it but looking at the examples makes me think it is a bad idea.

8 Likes

No, swift-format has no semantic knowledge—by design—so that it can operate on the syntax trees of source files without requiring their dependencies to be compiled. So for each of a function call's arguments, we only have access to the label (if present) and the argument value's expression, but nothing that says it was variadic vs. just an unlabeled argument slot.

So while it would be nice to be able to format variadic argument lists differently, perhaps by adding a line break and additional indentation:

func addSubviews(_ views: UIView..., constraints: NSLayoutConstraint...) { ... }

myView.addSubviews(
  v1,
  v2,
  constraints:
    myVeryVeryVeryVeryLooooooooooooooooongConstraint,
    myOtherVeryVeryLooooooooooongConstraintExpression
)

The call above would be syntactically indistinguishable from one where constraints was non-variadic and followed with a non-labeled argument. In that case—setting aside the odd signature and label—we may want to format them as traditional arguments:

func addSubviews(_ views: UIView..., constraints: NSLayoutConstraint, _ whatever: NSLayoutConstraint) { ... }

myView.addSubviews(
  v1,
  v2,
  constraints: myVeryVeryVeryVeryLooooooooooooooooongConstraint,
  myOtherVeryVeryLooooooooooongConstraintExpression
)

Now, one could potentially make the case that any labeled argument followed by one or more unlabeled arguments should be treated as a "related sequence" that should be formatted as in the first example. But since the syntax tree does not—and cannot—record whether the arguments are specifically for a variadic call, we'd have to make that decision universally and thus it could end up being applied to argument lists where it isn't the correct choice.

3 Likes

I like this proposal a lot, but the examples where a labelled Int is followed by an unlabelled variadic Int seemed very unreadable/unintuitive to me. I’m all for flexibility but not when it makes understanding code so hard.

Is there a reason not to require that a variadic that appears after a standard parameter of the same type must have a label?

-Wil

This would be a source-breaking change if it was applied to functions with a single variadic parameter, where it's sometimes used today as a way of requiring that at least one parameter is provided at the call site. For example:

func average(_ head: Double, _ tail: Double...) -> Double {
	let numbers = [head] + tail
	return numbers.reduce(0.0, +) / Double(numbers.count)
}

average() // error
average(1) // ok
average(1,2,3) // ok

I think rolling out this rule only for functions with >1 variadic parameter to avoid source breakage would create more confusion than it eliminated.

2 Likes

Ah, thank you.

I think this proposal makes sense, but I've got some reservations.

I'm not very keen on the examples - the assertArgs example tweaks my spidey sense because the two arrays are intimately linked but there is no way for the compiler to check that the second array makes any sense given the first. The addSubviews example is odd because the second array is literally just an array - there is no advantage to passing it as a variadic other than saving two brackets, but it comes with the disadvantage that you can't pass in a traditional array.

These are pretty bike-shedding level complaints but it contributes to my gut feeling is this is going to lead to APIs where programmers pass multiple arrays of interdependent data where a single array/variadic of a richer type would be more appropriate.

Given that the compiler can statically check how many parameters are provided, this could be part of the variadic syntax, for example 'variadic parameter that takes at least one but no more than 5 values'. I can't think of a syntax that isn't cumbersome though, and it goes beyond the scope of this proposal (and would still be a breaking change).

1 Like

+1 This hasn't been a pain point for me, but it adds consistency with other argument types and feels pretty non-controversial.

Review Conclusion

The review has ended and the proposal is accepted.

2 Likes