-
Notifications
You must be signed in to change notification settings - Fork 719
Allow ChannelCore implementors to unwrap NIOAny. #321
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
Conversation
Sources/NIO/NIOAny.swift
Outdated
| /// Try unwrapping the wrapped message as `ByteBuffer`. | ||
| /// | ||
| /// returns: The wrapped `ByteBuffer` or `nil` if the wrapped message is not a `ByteBuffer`. | ||
| @_versioned |
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.
@Lukasa why not @_inlineable as well?
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.
The current reason is that storage is private. Do we think that's worth changing?
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.
@Lukasa not for this particular one but possibly for the ones that take generics.
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.
Yeah, I'm a bit on the fence here. I suppose making the storage internal won't be too big a deal.
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.
me too tbh. I'll make it your call. If you make the storage internal, please underscore it
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.
Ok, made all of these @_inlineable.
320c382 to
b5f9d06
Compare
normanmaurer
left a comment
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.
Just one nit...
Sources/NIO/Channel.swift
Outdated
| } | ||
|
|
||
| public extension ChannelCore { | ||
| /// Unwraps the given NIOAny as a specific concrete type. |
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.
back-ticks around NIOAny
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.
Applied.
Motivation: When implementing a custom ChannelCore, you will probably need access to the data inside a NIOAny. It should be possible to unwrap that. Modifications: Added an extension to ChannelCore to allow unwrapping a NIOAny. Added @_versioned to almost all of NIOAny. Result: ChannelCore is implementable outside NIO
b5f9d06 to
5b11619
Compare
|
test failure should be addressed by #326 |
|
@swift-nio-bot test this please |
Motivation:
When implementing a custom ChannelCore, you will probably need access
to the data inside a NIOAny. It should be possible to unwrap that.
Modifications:
Added an extension to ChannelCore to allow unwrapping a NIOAny.
Added @_versioned to almost all of NIOAny.
Result:
ChannelCore is implementable outside NIO