SE-0330: Conditionals in Collections

I hear you but this leaves us precisely where we were three years ago. Given these constraints only a tactical, incremental change such as that in this proposal (fleshed out and rebased) is a way to take a step forward at this point. This would not preclude moving to the lexer at a later date as it would be functionality the same but there also seems to be a constituency of tooling users who would not be in favour of that anyway so why hold a proposal like this in limbo in perpetuity?

Returned for Revision

This review is circling around two points:

  1. Within the current syntax-based model for condition code, the proposal cannot be reasonably evaluated because the implications are not fully clear in both the written proposal and in the implementation (which is not up-to-date on main). Because the implementation is not up-to-date, the impact of the complexity of the implementation on the compiler is difficult to assess, and the behavior of various corner cases is not clear.

  2. Whether or not the syntax-based approach for conditional code should be abandoned entirely for a different approach, such as a lexer-based one. This creates and existential question of whether this proposed change should even be considered.

The review has waded into debating the second point. To that end, @allevato raised the following reasonable concern:

The core team concurs that such a significant change in direction should have its own properly considered pitch/discussion given the potentially seismic implications. A decision along these lines will not be made as part of this review.

However, the points raised in favor of a lexer-based approach contend that the complexity of the current approach continues to stack up, and is potentially unpredictable to the user. @Chris_Lattner3 framed this perspective succinctly:

In order for this proposal to be considered, the implications of the proposed change needs to be clear. There have been several questions that have been raised in the review that would need to be answered before accepting this proposal as a refinement to the current conditional code model. Thus, the proposal is returned for revision, with the following request:

  1. Update the implementation to work with main so that the implications of the implementation can be understood.

  2. The proposal speaks to the cases brought up here, and sorts through the broader concerns brought up. An updated implementation will be helpful to this end.

With an updated proposal and implementation, the community and core team can properly weigh the cost/benefit tradeoff of this refinement to the current approach, while also providing signal for a potential discussion of moving the conditional compilation to a lexer-based approach. To be clear, it is not a forgone conclusion that a lexer-based approach is an inevitable outcome. As @allevato points out, this deserves sufficient discourse to make such a call.

On behalf of the core team, I'd like to thank both @johnno1962 as the proposal author and everyone who participated in this discussion.

11 Likes

Thank you @tkremenek for bringing this to review even if the outcome wasn't entirely conclusive. In order to avoid situations like this in future could I suggest with appropriate temerity that we introduce a "pre-review" state into the Swift Evolution pipeline. There are 34 open PRs on the swift-evolution repo that could come to review at any time. Would it be possible to schedule proposals of interest to the core team formally by the act of merging the PR (automatically notifying the proposal author and allocating the SE-NNNN number) at least two weeks ahead of time. This would create a sort of "pre-review lobby" which could be on a dashboard somewhere where authors would be motivated to fine tune their proposal, ensure the implementation is up to date and receive final comments from the community before the proposal text is finalised.

All that said, I was able to reproduce a toolchain for the original implementation last night and can give definitive if rather belated answers to some of the questions that came up during the review. I'll not post the toolchain as it is from a time before ABI stability and only builds/works with Xcode 10.1.

This was marked as an error by the implementation. Internally, parsing of collection literals read ahead an expression and a token (skipping any conditionals) to see if the token was ':' or ',' to decide the dictionary/array-ness of the literal and goes down two distinct code paths once that has been determined.

In the original implementation which you can see from my reply to @rintaro's post it was decided to code it to require the trailing ',' inside conditionals. This wasn't strictly necessary and is probably a decision I'd revisit as it creates the need to add commas inside a conditional even if it is defines the last element of the collection.

3 Likes

Hi Ted,

Could the core team provide some guidance of whether the lexer based approach should also be considered? My read of your summary here is that you're asking for a better formed version of the array-only proposal. Is the core team interested in evaluating the larger problem?

-Chris

Terms of Service

Privacy Policy

Cookie Policy