Skip to content

Update legacy constructors, functions, and constants #1701

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 4 commits into from
Oct 15, 2018

Conversation

rauhul
Copy link
Member

@rauhul rauhul commented Sep 20, 2018

  • replaces *Make constructors with swift initializers
  • replaces NSZeroPoint/Size/Rect with .zero

@rauhul
Copy link
Member Author

rauhul commented Sep 20, 2018

@swift-ci please test

@spevans
Copy link
Contributor

spevans commented Sep 20, 2018

@swift-ci test

@rauhul
Copy link
Member Author

rauhul commented Sep 20, 2018

@parkera it doesn't seem like these failures are related to my changes. Is there something I'm missing?

@parkera
Copy link
Contributor

parkera commented Sep 20, 2018

Let's try again...

@parkera
Copy link
Contributor

parkera commented Sep 20, 2018

@swift-ci clean test

1 similar comment
@rauhul
Copy link
Member Author

rauhul commented Sep 22, 2018

@swift-ci clean test

@rauhul
Copy link
Member Author

rauhul commented Sep 22, 2018

@parkera it seems like swift-ci isn't launching new tests, any thoughts?

@rauhul
Copy link
Member Author

rauhul commented Sep 25, 2018

@swift-ci please test

@spevans
Copy link
Contributor

spevans commented Sep 25, 2018

@swift-ci test

@alblue
Copy link
Contributor

alblue commented Sep 26, 2018

Looks like related test failures on the Linux build; can you investigate?

03:11:36 TestFoundation/TestNSGeometry.swift:964: error: TestNSGeometry.test_NSUnionRect : XCTAssertTrue failed -
03:11:36 TestFoundation/TestNSGeometry.swift:965: error: TestNSGeometry.test_NSUnionRect : XCTAssertTrue failed -

@rauhul
Copy link
Member Author

rauhul commented Oct 3, 2018

So I've been having a pretty terrible time trying to get an ubuntu area working. Regardless I did some tests on macOS and I'm not even sure how the macOS test passed:

Welcome to Apple Swift version 4.2 (swiftlang-1000.11.37.1 clang-1000.11.45.1). Type :help for assistance.
  1> import Foundation
  2> let r1 = NSRect(x: CGFloat(1.2), y: CGFloat(3.1), width: CGFloat(10.0), height: CGFloat(10.0))
r1: NSRect = (origin = (x = 1.2, y = 3.1000000000000001), size = (width = 10, height = 10))
  3> let r2 = NSRect(x: CGFloat(10.2), y: CGFloat(2.5), width: CGFloat(5.0), height: CGFloat(5.0))
r2: NSRect = (origin = (x = 10.199999999999999, y = 2.5), size = (width = 5, height = 5))
  4> NSIsEmptyRect(NSRect.zero.union(NSRect.zero))
$R0: Bool = true
  5> NSEqualRects(r1, r1.union(NSRect.zero))
$R1: Bool = false
  6> NSEqualRects(r2, NSRect.zero.union(r2))
$R2: Bool = false
  7> NSIsEmptyRect(NSUnionRect(NSZeroRect, NSZeroRect))
$R3: Bool = true
  8> NSEqualRects(r1, NSUnionRect(r1, NSZeroRect))
$R4: Bool = true
  9> NSEqualRects(r2, NSUnionRect(NSZeroRect, r2))
$R5: Bool = true

I'm going to look into the difference between NSRect.zero and NSZeroRect on macOS to start.

@rauhul
Copy link
Member Author

rauhul commented Oct 3, 2018

@parkera
Copy link
Contributor

parkera commented Oct 3, 2018

Honestly, not sure about this one. You're saying the behavior is different on Darwin too?

@rauhul
Copy link
Member Author

rauhul commented Oct 3, 2018

Yeah it seems like they do behave differently. I tested with a small macOS command line application testapp.zip

Application output

Rectangles
r1: (1.2, 3.1, 10.0, 10.0)
r2: (10.2, 2.5, 5.0, 5.0)

CoreGraphics Implementations
r1 union zero: (0.0, 0.0, 11.2, 13.1)
zero union r2: (0.0, 0.0, 15.2, 7.5)

NSGeometry Implementations
r1 union zero: (1.2, 3.1, 10.0, 10.0)
zero union r2: (10.2, 2.5, 5.0, 5.0)

I guess I should revert the test changes, however I'm not sure how "Swift Test macOS Platform" passed.

@spevans
Copy link
Contributor

spevans commented Oct 3, 2018

@rauhul I think the "Swift Test macOS Platform" just compiles the code but it doesn't run the tests.

@rauhul rauhul force-pushed the update_legacy_functions branch from 2ac1091 to d7cfb68 Compare October 6, 2018 03:54
@rauhul
Copy link
Member Author

rauhul commented Oct 8, 2018

@swift-ci test

1 similar comment
@millenomi
Copy link
Contributor

@swift-ci test

@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

@swift-ci swift-ci merged commit 490d7a7 into swiftlang:master Oct 15, 2018
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.

6 participants