remove phantom publics from EventLoop and Heap #1298
Merged
+544
−492
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weissi
commented
Dec 9, 2019
| /// The whole processing of I/O and tasks is done by a `NIOThread` that is tied to the `SelectableEventLoop`. This `NIOThread` | ||
| /// is guaranteed to never change! | ||
| @usableFromInline | ||
| internal final class SelectableEventLoop: EventLoop { |
There was a problem hiding this comment.
this stuff is just moved apart from s/public/internal and fallout like @usableFromInline etc.
Lukasa
requested changes
Dec 10, 2019
Sources/NIO/EventLoop.swift
Outdated
| @@ -633,7 +633,8 @@ enum NIORegistration: Registration { | |||
| } | |||
|
|
|||
| /// Execute the given closure and ensure we release all auto pools if needed. | |||
| private func withAutoReleasePool<T>(_ execute: () throws -> T) rethrows -> T { | |||
| @inlinable | |||
| internal func withAutoReleasePool<T>(_ execute: () throws -> T) rethrows -> T { | |||
There was a problem hiding this comment.
Rather than make this internal and @inlinable, is there any reason not simply to move it to the file with its new usage?
There was a problem hiding this comment.
@Lukasa I can revert it but I didn't see a reason not to make it available in tests etc.
There was a problem hiding this comment.
@Lukasa also it now needs to be usableFromInline/inlinable which requires internal, I'll move it to the SelectableEventLoop file though
weissi
force-pushed
the
jw-rm-phantom-public
branch
from
2177013 to
b573f7e
Compare
Lukasa
approved these changes
Dec 10, 2019
Motivation: In SelectableEventloop, Heap, PriorityQueue, etc there were quite a few phantom publics left which cause issues with apple#1257 and are confusing. Modifications: - remove the phantom publics - move SelectableEventLoop to its own file Result: - clearer what's going on (public means public)
weissi
force-pushed
the
jw-rm-phantom-public
branch
from
b573f7e to
2b80716
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
In SelectableEventloop, Heap, PriorityQueue, etc there were quite a few
phantom publics left which cause issues with #1257 and are confusing.
Modifications:
Result: