Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[stdlib] DoubleWidth refinements #11161

Merged
merged 5 commits into from
Aug 2, 2017
Merged

[stdlib] DoubleWidth refinements #11161

merged 5 commits into from
Aug 2, 2017

Conversation

natecook1000
Copy link
Member

This adds tests for masking bit shifts to DoubleWidth and addresses the feedback from @dabrahams here: #10131

@natecook1000
Copy link
Member Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 7e5c78c
Test requested by - @natecook1000

@natecook1000
Copy link
Member Author

@swift-ci Please test OS X platform

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 7e5c78c
Test requested by - @natecook1000

@natecook1000
Copy link
Member Author

@swift-ci Please test OS X platform

@natecook1000 natecook1000 requested review from moiseev and removed request for dabrahams July 27, 2017 00:58
@moiseev
Copy link
Contributor

moiseev commented Jul 27, 2017

Can we please wait till #11203 lands? Please?

moiseev
moiseev reviewed Jul 27, 2017
View reviewed changes
stdlib/public/core/DoubleWidth.swift.gyb Outdated
(rhs._storage.low - numericCast(Base.bitWidth)))
}

let t = Base.bitWidth > rhs._storage.low
Copy link
Contributor

Choose a reason for hiding this comment

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

t for temporary?

moiseev
moiseev suggested changes Jul 27, 2017
View reviewed changes
Copy link
Contributor

@moiseev moiseev left a comment

Choose a reason for hiding this comment

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

LGTM, apart from the t variable name. Maybe newLow and newHigh would read better?

@natecook1000
Copy link
Member Author

@swift-ci Please smoke test

moiseev
moiseev approved these changes Aug 2, 2017
View reviewed changes
Copy link
Contributor

@moiseev moiseev left a comment

Choose a reason for hiding this comment

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

:shipit:

@natecook1000 natecook1000 merged commit 6a72622 into swiftlang:master Aug 2, 2017
natecook1000 added a commit to natecook1000/swift that referenced this pull request Aug 3, 2017
@natecook1000
Merge pull request swiftlang#11161 from natecook1000/nc-dwformatting
a05c56c 
[stdlib] DoubleWidth refinements
@natecook1000 natecook1000 deleted the nc-dwformatting branch October 4, 2018 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moiseev moiseev moiseev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@natecook1000 @swift-ci @moiseev