[Sema] ban multi-arguments to tuple coercion #3895
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dduan
changed the title
dduan
force-pushed
the
se0110_a-pr
branch
3 times, most recently
from
746f067 to
9939388
Compare
This was referenced Jul 31, 2016
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
the Foundation build failure has been addressed in this PR: swiftlang/swift-corelibs-foundation#497 |
Contributor
|
Nice! AFAICT we can use this patch to sniff out problems in the dependencies first and merge this after they're fixed, right? |
Contributor
Author
|
@CodaFi correct. I've done so with Foundation and XCTest. |
Contributor
|
Has this been tested against lldb, swift-playgroundlogger, and swiftpm? |
Contributor
Author
|
@CodaFi ran test on lldb, got this. Are the unexpected success normal? |
Contributor
|
I'm not sure about the tests, but that it builds is a positive sign WRT this patch. |
Contributor
Author
|
swiftpm changes: swiftlang/swift-package-manager#581 |
Contributor
|
And a quick rebase please. |
Contributor
Author
|
@CodaFi I tried to test playground logger with |
Implements part of SE-0110. Single argument in closures will not be accepted if
there exists explicit type with a number of arguments that's not 1.
```swift
let f: (Int, Int) -> Void = { x in } // this is now an error
```
Note there's a second part of SE-0110 which could be considered additive,
which says one must add an extra pair of parens to specify a single arugment
type that is a tuple:
```swift
let g ((Int, Int)) -> Void = { y in } // y should have type (Int, Int)
```
This patch does not implement that part.
Contributor
Author
|
@CodaFi rebased |
Contributor
./swift/utils/build-script --playgroundlogger <FURTHER-OPTIONS>Should do it. |
Contributor
Author
|
@CodaFi Thanks! playgroundlogger is in good shape. No change needed. |
Member
|
The Swift changes look fantastic; thanks for pushing this through! |
Contributor
Author
|
Here's lldb test results ran on master. I think the unexpected success is part of the deal (except the count is different?) |
Contributor
|
You don't have to worry about that here. What I meant was more that there are test cases inside LLDB that run swift code that may need migration. But if the suite builds without hard compiler errors then your job is done. |
dduan
changed the title
Contributor
|
@swift-ci please smoke test. |
Member
|
@swift-ci please smoke test OS X |
Member
|
the ErrorProtocol issue is something I need to investigate; re-running tests. |
Contributor
|
@DougGregor Can you disable that test using a requires while you investigate? |
Member
|
This caused a pretty serious regression. We no longer type-check this: |
Contributor
Author
would work, no? |
Member
|
It shouldn't work. The element type is (Int, Int), and we should be passing that as a single argument to the closure, not splatting it out to two arguments. |
Member
|
It's probably falling into the "second part of SE-0110" that isn't yet implemented. It might be that we simple cannot separate the two parts. |
Contributor
Author
|
I see. Let's revert this then. |
Contributor
Author
|
I'll work on the second part tonight. |
MnO2
added a commit
to MnO2/swift
that referenced
this pull request
Aug 14, 2016
This commit built upon the work of Pull Request 3895. Apart from the
work to make the following work
```swift
let f: (Int, Int) -> Void = { x in } // this is now an error
```
This patch also implement the part 2 mentioned in the swiftlang#3895
```swift
let g: ((Int, Int)) -> Void = { y in } // y should have type (Int, Int)
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's in this pull request?
Implements part of SE-0110. Single argument in closures will not be accepted if
there exists explicit type with a number of arguments that's not 1.
Note there's a second part of SE-0110 which could be considered additive,
which says one must add an extra pair of parens to specify a single arugment
type that is a tuple:
This patch does not implement that part.
Edit:
Updates for Foundation, XCTest and SwiftPM:
swiftlang/swift-corelibs-foundation#497
swiftlang/swift-corelibs-xctest#149
swiftlang/swift-package-manager#581
Resolved bug number: (SR-2008, partial)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.