Skip to content

ContiguousCollection protocol revisited #990

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
Merged
merged 9 commits into from
Apr 3, 2019
Merged

ContiguousCollection protocol revisited #990

merged 9 commits into from
Apr 3, 2019

Conversation

airspeedswift
Copy link
Member

A re-pitch of the protocols dropped from SE-0237, with motivating example.

@airspeedswift airspeedswift added the standard library addition Additive changes to the standard library label Feb 27, 2019
@airspeedswift airspeedswift changed the title [stdlib] ContiguousCollection protocol revisited ContiguousCollection protocol revisited Feb 27, 2019
@fassko fassko mentioned this pull request Feb 27, 2019
Closed
stephentyrone
stephentyrone reviewed Feb 27, 2019
View reviewed changes
stephentyrone
stephentyrone requested changes Feb 27, 2019
View reviewed changes
Copy link
Contributor

@stephentyrone stephentyrone left a comment •
edited
Loading

Choose a reason for hiding this comment

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

vDSP_vsaddi is a bad function to use here because B is actually a scalar. You want to use vDSP_vaddi throughout (or maybe vDSP_vadd and change element types to Float to maximize familiarity for reviewers; the float functions are much more idiomatic in vDSP).

@airspeedswift
Copy link
Member Author

Oops in fact that was a typo, I meant vDSP_vaddi. The fact that they both take a pointer is super cursed. Switched to the Float version.

@stephentyrone
Copy link
Contributor

Fortran. Boy, I don’t know.

moiseev
moiseev reviewed Feb 27, 2019
View reviewed changes
moiseev and others added 2 commits February 27, 2019 20:53
@airspeedswift airspeedswift marked this pull request as ready for review March 6, 2019 18:42
tkremenek
tkremenek reviewed Mar 26, 2019
View reviewed changes
@airspeedswift airspeedswift mentioned this pull request Mar 28, 2019
Closed
@tkremenek tkremenek merged commit 9655626 into swiftlang:master Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tkremenek tkremenek tkremenek left review comments

@stephentyrone stephentyrone stephentyrone approved these changes

+1 more reviewer

@moiseev moiseev moiseev left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

standard library addition Additive changes to the standard library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@airspeedswift @stephentyrone @moiseev @tkremenek