Skip to content

[Swift 2.2.1] [SILGen] Fix for 'try?' leaking the error value #1886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 30, 2016

Conversation

Reflejo
Copy link

@Reflejo Reflejo commented Mar 26, 2016

What's in this pull request?

Backport the fix for try? memory leakage to Swift 2.2. This was the original PR to master: #1740. @tkremenek please consider merging it for a 2.2.x release.

Resolved bug number: (SR-972)


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

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

Platform Comment
All supported platforms @swift-ci Please smoke test
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
OS X platform @swift-ci Please test OS X platform
Linux platform @swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

@Reflejo Reflejo changed the title Optional try leak SILGen: Fix for 'try?' leaking the error value Mar 26, 2016
@tkremenek
Copy link
Member

@swift-ci test

@Reflejo Reflejo force-pushed the optional-try-leak branch from 92f3379 to d38dbc1 Compare March 27, 2016 03:06
@Reflejo Reflejo changed the title SILGen: Fix for 'try?' leaking the error value [Swift 2.2.1] [SILGen] Fix for 'try?' leaking the error value Mar 27, 2016
@tkremenek
Copy link
Member

@swift-ci test


ErrorHandlingTests.test("tryOptional") {
expectEqual(LifetimeTracked(1), try? furball(false))
expectEqual(Optional<LifetimeTracked>.none, try? furball(true))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is written for Swift 3. I believe changing it to .None will make the test pass.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, missed that one.

@Reflejo Reflejo force-pushed the optional-try-leak branch from d38dbc1 to 563859b Compare March 28, 2016 17:59
@Reflejo
Copy link
Author

Reflejo commented Mar 28, 2016

@tkremenek Tests should be passing now, apologies for the noise

@tkremenek
Copy link
Member

@swift-ci test

@Reflejo Reflejo force-pushed the optional-try-leak branch from 563859b to 3d82ffd Compare March 29, 2016 01:46
@Reflejo
Copy link
Author

Reflejo commented Mar 29, 2016

Test passed, I had to rebase after: #1798 and #1919 though.

@Reflejo Reflejo force-pushed the optional-try-leak branch from 3d82ffd to 4349b89 Compare March 29, 2016 06:55
@tkremenek
Copy link
Member

@swift-ci test and merge

@tkremenek tkremenek added this to the Swift 2.2.x milestone Mar 29, 2016
@tkremenek tkremenek self-assigned this Mar 29, 2016
@dduan
Copy link
Contributor

dduan commented Mar 30, 2016

Apparently test and merge doesn't work on protected branch like this one.

cc @shahmishal

@tkremenek
Copy link
Member

@swift-ci test

@tkremenek
Copy link
Member

Makes sense for test and merge not to work. Not a problem. I'll merge directly.

@tkremenek tkremenek merged commit 41c313b into swiftlang:swift-2.2-branch Mar 30, 2016
MaxDesiatov pushed a commit that referenced this pull request Oct 13, 2020
…-assertions

Disable assertions for both target and host toolchain in 5.3 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants