SR-11283: Amend OptionSet.remove method #28378

JacobMao · 2019-11-20T05:03:24Z

This PR modify OptionSet.remove method to make it conform to the description of the document.
Currently this method returns wrong result when element intersects but is not fully contained.
I also added some test cases for this.

Resolves SR-11283.

Update upstream

PR

Update

Merge

ddd

merge

PR

merge

JacobMao · 2019-11-20T05:12:29Z

cc @lilyball @lorentey

JacobMao · 2019-11-20T05:16:02Z

@swift-ci test

stdlib/public/core/OptionSet.swift

CodaFi · 2019-11-20T23:45:26Z

@JacobMao Only those with the commit bit are allowed to trigger PR testing for security reasons.

Could you please squash the merge commits out of this pull request and just leave behind a single commit with your changes?

JacobMao · 2019-11-21T01:17:23Z

@CodaFi
thanks for your review. sorry, I forgot to squash those commits.
I will do it immediately

CodaFi · 2019-11-21T01:59:41Z

I think it's still not quite right. You can run git rebase (I prefer an interactive one with -i) and delete these commits. If you don't want them to appear in the future, you can always use git pull --rebase <target> <branch>

lorentey

Thank you! I requested a slight coding style change, but otherwise this looks ready to land.

I'll be able to squash these commits when I merge this -- don't worry too much about removing them.

stdlib/public/core/OptionSet.swift

lorentey · 2019-11-21T03:50:09Z

@swift-ci test

lorentey · 2019-11-21T04:02:35Z

Note: To help us manage compatibility risks, we'll need to delay merging this PR until the 5.2 branch is cut. (Currently scheduled for December 9th.)

JacobMao · 2019-11-21T06:00:09Z

@swift-ci test

CodaFi · 2019-11-21T06:01:18Z

@swift-ci please test

swift-ci · 2019-11-21T06:04:58Z

Build failed
Swift Test OS X Platform
Git Sha - d5e2533

CodaFi · 2019-11-21T06:06:38Z

This seems like an infrastructure problem. Maybe the smoke test bots are having a better time

@swift-ci please smoke test macOS platform

CodaFi · 2019-11-21T07:25:18Z

Ugh, it's still happening. Ignore the macOS build. We'll get that sorted tomorrow.

CodaFi · 2019-11-21T16:49:02Z

@swift-ci please clean test macOS platform

swift-ci · 2019-11-21T18:25:28Z

Build failed
Swift Test OS X Platform
Git Sha - d5e2533

CodaFi · 2019-11-21T23:39:14Z

Ah, this picked up Doug's failure from this morning. Let's try one more time.

@swift-ci please clean test macOS platform

lorentey · 2019-12-10T20:32:48Z

@swift-ci test

lorentey · 2019-12-11T01:43:49Z

Oh, alas, I forgot to squash the commits. Sorry about that!

shahmishal · 2019-12-11T20:15:40Z

We are seeing test failure after merging this PR:

Swift(macosx-x86_64) :: stdlib/OptionSetTest.swift

shahmishal · 2019-12-11T20:16:11Z

https://ci.swift.org/job/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/923/consoleFull

20:36:47 ******************** TEST 'Swift(macosx-x86_64) :: stdlib/OptionSetTest.swift' FAILED ********************
20:36:47 Script:
20:36:47 --
20:36:47 : 'RUN: at line 12';   rm -rf "/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp" && mkdir -p "/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp" && xcrun --toolchain default --sdk '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk' /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/bin/swiftc -target x86_64-apple-macosx10.9  -module-cache-path '/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache' -F '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks' -toolchain-stdlib-rpath -Xlinker -rpath -Xlinker '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks'  -swift-version 4  -Xfrontend -ignore-module-source-info  -F '/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib' -Xlinker -rpath -Xlinker '/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib' -module-cache-path '/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache' /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/swift/test/stdlib/OptionSetTest.swift -o /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp/a.out -module-name main  && codesign -f -s - /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp/a.out &&/usr/bin/env DYLD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx' LD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx:' SIMCTL_CHILD_DYLD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx'  /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp/a.out
20:36:47 --
20:36:47 Exit Code: 134
20:36:47 
20:36:47 Command Output (stdout):
20:36:47 --
20:36:47 [ RUN      ] OptionSet.basics
20:36:47 [       OK ] OptionSet.basics
20:36:47 [ RUN      ] OptionSet.set algebra
20:36:47 stdout>>> check failed at /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/swift/test/stdlib/OptionSetTest.swift, line 98
20:36:47 stdout>>> expected: Optional(main.PackagingOptions(rawValue: 4)) (of type Swift.Optional<main.PackagingOptions>)
20:36:47 stdout>>> actual: nil (of type Swift.Optional<main.PackagingOptions>)
20:36:47 [     FAIL ] OptionSet.set algebra
20:36:47 OptionSet: Some tests failed, aborting
20:36:47 UXPASS: []
20:36:47 FAIL: ["set algebra"]
20:36:47 SKIP: []
20:36:47 To debug, run:
20:36:47 $ /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp/a.out --stdlib-unittest-in-process --stdlib-unittest-filter "set algebra"
20:36:47 
20:36:47 --
20:36:47 Command Output (stderr):
20:36:47 --
20:36:47 /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.script: line 1: 55283 Abort trap: 6           /usr/bin/env DYLD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx' LD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx:' SIMCTL_CHILD_DYLD_LIBRARY_PATH='/usr/lib/swift:/Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/lib/swift/macosx' /Users/buildnode/jenkins/workspace/oss-swift_tools-RA_stdlib-RA-test-simulator-swift-in-the-os/buildbot-custom/swift-macosx-x86_64/test-macosx-x86_64/stdlib/Output/OptionSetTest.swift.tmp/a.out
20:36:47 
20:36:47 --
20:36:47 
20:36:47 ********************

lorentey · 2019-12-11T20:24:23Z

Ah, we need to add availability checks to the new tests, or they will keep failing on back deployment tests. I'll take care of it.

JacobMao added 18 commits November 12, 2017 15:54

Merge pull request #1 from apple/master

bc50c03

Update upstream

Merge pull request #2 from apple/master

53287af

PR

Merge pull request #3 from apple/master

4fe24e3

Update

Merge pull request #4 from apple/master

7fc998c

Merge

Merge pull request #5 from apple/master

7fb16b8

ddd

Merge pull request #6 from apple/master

0089ad4

merge

Merge pull request #7 from apple/master

d5b2ef5

merge

Merge pull request #8 from apple/master

16addd9

merge

Merge pull request #9 from apple/master

9274bd1

merge

Merge pull request #10 from apple/master

90b06be

merge

Merge pull request #11 from apple/master

a3fbb5f

merge

Merge pull request #12 from apple/master

a72b451

merge

Merge pull request #13 from apple/master

dfe4d5f

PR

Merge pull request #14 from apple/master

4e65573

merge

Merge pull request #15 from apple/master

b495847

merge

Merge pull request #16 from apple/master

1f0a78e

merge

Merge pull request #17 from apple/master

c6c0a6d

merge

Merge pull request #18 from apple/master

2570edd

merge

lilyball reviewed Nov 20, 2019

View reviewed changes

stdlib/public/core/OptionSet.swift Show resolved Hide resolved

CodaFi requested a review from lorentey November 20, 2019 23:45

Merge branch 'master' of https://github.com/JacobMao/swift

ac3a528

JacobMao added 2 commits November 21, 2019 11:25

fixed OptionSet's remove method

a8fd973

added test case for OptionSet's set algebra methods

f5ef37a

JacobMao force-pushed the fix/SR-11283 branch from bd7d146 to f5ef37a Compare November 21, 2019 02:28

lorentey approved these changes Nov 21, 2019

View reviewed changes

stdlib/public/core/OptionSet.swift Show resolved Hide resolved

This comment has been minimized.

Sign in to view

lorentey added the pending convergence label Nov 21, 2019

improved OptionSet.remove's code style

d5e2533

swiftlang deleted a comment from swift-ci Nov 21, 2019

lorentey self-assigned this Nov 22, 2019

lorentey removed the pending convergence label Dec 10, 2019

lorentey merged commit 2e17867 into swiftlang:master Dec 11, 2019

This was referenced Dec 11, 2019

[test] Protect OptionSet.remove test with future availability #28715

Merged

[5.2][stdlib] Don’t use assert() in the stdlib #28730

Merged

lorentey mentioned this pull request Jan 25, 2020

Stdlib doc changes #29436

Merged

natecook1000 mentioned this pull request Sep 8, 2020

Fixes example snippet in OptionSet.swift #33846

Closed

swift-ci mentioned this pull request Oct 31, 2019

[SR-11283] OptionSet.remove returns wrong result when element intersects but is not fully contained #53684

Closed

SR-11283: Amend OptionSet.remove method #28378

SR-11283: Amend OptionSet.remove method #28378

Uh oh!

Conversation

JacobMao commented Nov 20, 2019

Uh oh!

JacobMao commented Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JacobMao commented Nov 20, 2019

Uh oh!

Uh oh!

CodaFi commented Nov 20, 2019

Uh oh!

JacobMao commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorentey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lorentey commented Nov 21, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

lorentey commented Nov 21, 2019

Uh oh!

JacobMao commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019

Uh oh!

swift-ci commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019

Uh oh!

swift-ci commented Nov 21, 2019

Uh oh!

CodaFi commented Nov 21, 2019

Uh oh!

lorentey commented Dec 10, 2019

Uh oh!

lorentey commented Dec 11, 2019

Uh oh!

shahmishal commented Dec 11, 2019

Uh oh!

shahmishal commented Dec 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorentey commented Dec 11, 2019

Uh oh!

Uh oh!

JacobMao commented Nov 20, 2019 •

edited

Loading

CodaFi commented Nov 21, 2019 •

edited

Loading

shahmishal commented Dec 11, 2019 •

edited

Loading