-
Notifications
You must be signed in to change notification settings - Fork 483
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
Added missing explicitly named operator equivalents #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Providing in-place variants for these operations is a great idea. The only concern I have is that the use of ___InPlace
may pollute the top-level namespace unnecessarily. Instead of that suffix, what do you think of a spelling that overloads these with the regular versions of these functions? For example, public func add<L: UnsafeMutableMemoryAccessible, R: UnsafeMemoryAccessible>(_ lhs: inout L, _ rhs: R)
? Would that work, or am I missing something obvious about UnsafeMutableMemoryAccessible
that makes this approach untenable?
I used Just like we have Other prior art:
|
As I understand it this PR is blocking the others, right? Calling things |
(Really sorry for dropping the ball on this thread. I finally have another chance to work on this.) @regexident Yeah, I made the wrong choice when I originally named the APIs (though to be fair, it predates the Swift API Design Guidelines). While it'd be idea for us to adopt a consistent Maybe we can revisit this with the next major version, but these changes are fine for now. The only other idea I might offer at this point is, maybe we can sidestep the issue by making the named functions themselves |
This sounds like a good compromise to me! 👍 I'll make them |
With this PR operators now are strictly forwarding functions.
Once https://github.com/mattt/Surge/pull/105 is merged I'll do a rebase and add unit tests for the remaining
inout
/…inPlace
functions.