1

The core team is going through the backlog of proposed evolution proposals, and would like to put forward SE-0330: Conditionals in Collections for review by the community. The review begins now and runs through November 19th, 2021.

The proposal is authored by @johnno1962.

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

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • 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?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to Swift.

Ted Kremenek
Review Manager

12 Likes
2

I'm rather baffled about this particular proposal coming to review.

The pitch thread was some (over three?) years ago and basically ended with a note from @Chris_Lattner3 summarizing the core team's decision that a larger discussion surrounding #if was called for as the original design goals are now obsolete. I don't recall seeing any further discussion on that point.

I do agree, as I did then, that the addition itself is small and seemingly obvious. During the pitch phase, several questions about whether certain corner cases ought or ought not to be accepted were posed. The proposal, as presented today for review, does not answer those questions even years later. Specifically:

It's unclear where the proposal comes down re @rintaro's points in this post regarding whether commas are necessary in those examples.

Additionally, there's never been an answer regarding this example:

Moreover, these questions are not answerable by using a toolchain with a draft implementation to test the proposed behaviors, since there isn't one (the referenced PR has been closed for a long time).

So my overall evaluation would be that I would love to see some progress in this area, and the problem addressed is significant enough that an unobtrusive, limited addition to the language would be warranted. Indeed, the examples given in the proposal do fit well with the feel and direction of Swift; however, these examples do not completely specify an implementable result, and the original implementation caused concern to the core team and is now long since obsolete.

I have obviously used corresponding features in other C-family languages; this is a nice step towards adding some of the flexibility in those languages but, as the core team judged many years ago, a wholesale reconsideration is due here. I read this iteration of the proposal thoroughly and went back to study the prior thread as well as the PR that's now closed.

11 Likes
3

Good except for the name and the formatting. The thread got those both right.

e.g. SIMD is not a Collection.

let expressedByArrayLiteral: SIMD3 = [
  0,
  #if os(macOS)
  1,
  #else
  2,
  #endif
  3
]
4

Thanks @xwu for bringing up this point.

The core team took a fresh look at this proposal. We agreed it should go for review based on considering the current implementation of these conditionals. This is similar to what we did with SE-0308, where we did decide to extend the model within the current framework. While moving #if support into the lexer remains an interesting potential direction, no real momentum has come forth on such a direction since the time this proposal was put forth. We thus felt, when we revisited this proposal, that it should be considered by the community on its own merits within the model that currently exists. I should have made this clear when the review was launched.

5

As with SE-0308, I am -1 on this proposal.

This continues to make the language model more complicated by adding special cases. SE-0308 was motivated by rushing in a "quick fix" for critical SwiftUI cases before WWDC. I don't see why we would continue to exacerbate the problems here, instead of investing in a properly scalable fix.

As a procedural issue, is there an implementation of this? The proposal has a 3-year old abandoned patch.

-Chris

7 Likes
6

:upside_down_face:

7

I'm not sure why you're quoting that out of context John. The exact thread you cite shows a good discussion which led to the other conclusion downthread. In any case, that thread is three years old and many conversations have happened between then and now.

5 Likes
8

Sorry Chris, I apologise but I couldn't resist it. That comparatively short thread and in particular your later comment is indeed required reading for commenting on this review. I personally am still in favour of this pragmatic, incremental change as there is a use case for conditional test inclusion and I think a more full blown evaluation of moving Swift's preprocessor into the lexer is unlikely for the foreseeable (even though I'd fully support that and would even look at it in terms of the compiler if I thought there was backing - proceeding with this proposal would not preclude looking at the lexer later).

9

The patch is indeed outdated.

From the existing patch, I believed that there was sufficient signal of what the impact would be to the compiler's implementation (the basis of some of the concerns here) and the complexity of the language. Historically, there's been some amount of flexibility on the degree of completeness of implementations when evaluating proposals, depending on the complexity of the feature, etc. I made the call that the existing patch was sufficient to consider the proposal as is for review instead of leaving it in stasis or asking @johnno1962 to put in more work to update the patch when (I believed) there was sufficient signal already to evaluate the proposal. However, if there is doubt from the community that, if the patch were to be updated to top of main, that the implementation would be so different now that the proposal cannot be properly evaluated (both to its impact to the language model or compiler implementation) then that feedback is welcome in this review. That feedback can either on this thread or to direct message to me as the review manager. Based on that feedback, the course with this review can be adjusted accordingly.

2 Likes
10

This feels similar to the proposal to add Equatable/Hashable/Comparable conformance to tuples (in terms of letting the perfect be the enemy of the good). My review is basically the same as it was for that proposal:

  • Is the feature worth adding to the Swift language? Clearly yes.

  • Is it worth the technical debt until a more comprehensive solution comes along? IMO, that’s a question only the compiler team can really answer.

17 Likes
11

Thank you for bringing up that question. At first, I personally don't think this will be a technical debt, but rather a natural addition to the current model. In the initial core team's decision, Chris raised a concern:

In SE-0308, we implemented the feature without modifying AST at all. it's only Parser and Syntax change. SE-0308: Postfix '#if' expression by rintaro · Pull Request #35097 · apple/swift · GitHub Before the introduction of libSyntax, we needed to store all #if...#endif information in AST so we can do syntactic coloring etc. even in non-active #if blocks . Now that we have Syntax. Since AST doesn't need to hold syntactic information anymore, adding a use of #if ... #endif is rather simple now. Syntactic tooling like swift-format can support this feature by using the Syntax tree and it'll will probably be an additive change.

2 Likes
12

Right, I agree that it would be much better to do such things in Syntax instead of the AST, but this isn't what the proposal is proposing. Further, as Xiaodi points out, there are still unanswered questions with the language model, e.g. are things like this allowed?

let collection = [
#if FLAG
  "foo",
#else
  "foo": 12,
#endif
]

We can't evaluate this because there isn't a working implementation available.

-Chris

1 Like
13

SE-0308 was additive because it was a completely new expression node type that fit into the existing ExprSyntax hierarchy. But I don't think this change could be strictly additive for swift-format (or any SwiftSyntax user) because the elements of ArrayElementListSyntax are currently defined to only be ArrayElementSyntax. We'd have to make a breaking change somewhere in the structure, like raising the type of the elements of ArrayElementListSyntax to the type-erased Syntax wrapper so that it could contain either IfConfigDeclSyntaxes or ArrayElementSyntaxes.

This example is even scarier, because in a regular array/dictionary literal, the syntax node that encompasses the square brackets is already typed as an ArrayExprSyntax or DictionaryExprSyntax. If this example were allowed, neither of those would be satisfiable; we'd have to invent something new like an ArrayOrDictionary🤷🏻‍♂️ExprSyntax to support it.

IMO, I can't think of a non-contrived situation where being able to do the above array/dictionary dance is desirable. The goal of this proposal is to simplify conditionally including/excluding elements from a collection, not to allow a collection to conditionally be either an array or dictionary; I don't think we should allow the latter and if it were allowed I think the complexity would far outweigh the benefit. If an #if is the first thing we hit after a [, I would just determine whether the collection is an array or dictionary based on the first non-#if content that I see, and error out if any of the other blocks don't match that determination.

I'd also like to see some examples involving conditionally empty dictionaries to understand the requirements around how they're expressed, since they have the special [:] syntax. For example, would this be allowed because the content of the #if-block would provide enough context that the whole expression is a dictionary literal?

let dictionary = [
  #if FLAG
    "foo": 10,
    "bar": 20,
  #endif
]

Or would this need to be expressed like this instead, because omitting the #if-block would be equivalent to []?

let dictionary = [
  #if FLAG
    "foo": 10,
    "bar": 20,
  #else
    :
  #endif
]

And then how do you square that with more complex cases?

let dictionary = [
  #if FLAG1
    "foo": 10,
    "bar": 20,
  #else
    :
  #endif
  #if FLAG2
    :
  #else
    "meep": 30,
    "yeex": 40,
  #endif
]

I'm supportive of the ideas in this proposal in general, but like others in the thread, I think the write-up needs to be updated a good deal and the implementation needs to be refreshed before it's able to be reviewed.

8 Likes
14

Another semi-related question to those already raised is what the element-type joining behavior looks like for collection literals with #if conditionals inside:

class B {}
class C: B {}

// is arr of type [B], or depends on FLAG?
let arr = [
  #if FLAG
    C(),
  #else
    B(),
]

(I think this can only reasonably result in the type of arr being dependent on FLAG since expressions in the branch-not-taken needn't even compile, but it would be good to spell that out explicitly)

8 Likes
15

You are right the current Array/DictionaryElementSyntax cannot hold IfConfigDeclSyntax. However, we can make it additive if we introduce an Expr syntax which wraps IfConfigDeclSyntax. Since the comma in Array/DictionaryElementSyntax is optional, we could model e.g.

[
  value,
#if CONFIG
  value,
#endif
  value,
]

like

(ArrayLiteralSyntax
  (ArrayElementListSyntax
    (ArrayElementSyntax
      (IdentifierExprSyntax)
      <comma>)
    (ArrayElementSyntax
      (IfConfigCollectionElementExprSyntax
        (IfConfigDeclSyntax ... ))
      <comma=nil>))
    (ArrayElementSyntax
      (IdentifierExprSyntax)
      <comma>))))

Of course this is just an idea.

raising the type of the elements of ArrayElementListSyntax to the type-erased Syntax wrapper

may be a better option. We should discuss more about this.

16

A big -1 for this proposal. IMO the fact that #if in Swift is processed at syntax pass instead of lexical pass is a pain point to the compiler team in terms of maintenance, as well as a big surprise to programmers from the C language family.

If the #if should be processed at the lexical level (I think so), then patches like this proposal one after another are just hacky solutions to fix holes in the language. Not mentioning the additional test and potential compatibility cost when we finally make the shift.

If we think that #if should be processed at the syntax level (as it is today), then at least we could have a thorough discussion about all the pros and cons. This proposal is not urgent as far as I can see now.

17

This is a really great example. Note that (regardless of the shape of the final proposal) Swift doesn't (and can't) type check things in #if false regions. This means we can't know that the former is String to Int dictionary. The means that even the later can't work when FLAG is false, because we don't know the key/value types for the type of dictionary.

Trying to "make this work" in some cases would lead to a really fragile and unpredictable model that doesn't compose well.

One of the reasons that I'm in favor of a simple lexical #if model is that it is simple, explainable, and predictable for users. Our current model is already a pile of non-obvious special cases.

-Chris

2 Likes
18

Here is my take on the matter by way of "rerunning" some of the above examples using a lightweight syntax and bending some of the current rules a bit here and there:

let dictionary: [String: Int] = [
	#if flag {
		"foo": 10,
		"bar": 20
	}
]

let dictionary: [String: Int] = [] // allow this spelling when obvious to the compiler

let dictionary: [String: Int] = [
	#if flag1 {
		"foo": 10,
		"bar": 20
	}
	#if flag2 {
		// nothing here
	} else {
		"meep": 30,
		"yeex": 40
	}
]

let arr: B [
	#if flag {
		C()
	}
	B()
]

[
	value1,
	#if config {
		value2
	}
	value3
]

let dictionary: [String: Int] = [
	#if condition {
		"foo": 10,
		"bar": 20
	}
	if #available(iOS 14, *) {
		"baz": 10
	}
}

and no to:
let collection = [
#if flag {
"foo"
} else {
"foo": 12
}
]

to sum up:

  • a lightweight #if condition { ... } syntax vs more heavy current #if ... #endif syntax.
  • require explicit type annotation when it would be otherwise ambiguous or unclear - don't think it's a much of a burden.
  • relax some rules about punctuation to avoid visually disturbing commas
  • allow [ ] used instead of [:] when it is obvious to the compiler what's going on.
  • prohibit insane cases when type is changed between array and dictionary
  • and give #available the similar treatment (like in the last example)
19

It seems inevitable this proposal will be "returned for revision" to update the implementation and flesh out details of how various edge cases should be handled. I have tried over the last two days to build a toolchain of the original patch (as it can not easily be rebased) but this has eluded me so I can't give you anything more concrete to go on after all this time.

All of this is irrelevant however if a firm decision has already been made that the processing of preprocessor conditionals should be moved into the lexer which if this were the case, can this be communicated in the review decision and can someone be assigned to start looking at it please? In this way it can be made clear whether there is any point/need to pursue the proposal further.

1 Like
20

Since this would have considerable usability and compatibility impact on SwiftSyntax users like swift-format and others writing Swift source analysis tooling, I hope that such a proposed change would be given its own pitch/discussion thread and not be communicated as part of this review decision.

IMO, despite the limitations around where #if blocks can occur in the language, I think making them part of the syntax tree is still a strength of Swift's syntax model and mimicking C's preprocessor would be a mistake.

9 Likes