Skip to content

Conversation

@MrMage
Copy link
Collaborator

@MrMage MrMage commented Apr 5, 2018

To support default arguments for the timeout, we need to implement this in a convoluted way with a trampoline in a protocol extension. This is because protocol methods are not allowed to provide default argument values, but protocol extensions are.

To support default arguments for the timeout, we need to implement this in a convoluted way with a trampoline in a protocol, as protocol methods are not allowed to provide default argument values.
internal protocol Echo_EchoExpandCall: ClientCallServerStreaming {
/// Call this to wait for a result. Blocking.
func receive() throws -> Echo_EchoResponse?
/// Do not call this directly, call `receive()` in the protocol extension below instead.
Copy link
Member

Choose a reason for hiding this comment

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

Can this be clarified with "private" access control?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wish I could, but protocols can't have access modifiers for individual methods. And given that the extension (with the default arguments) should be public, the helper method needs to be, too :-( If someone knows a better way to do this, I'd love to change it!

Copy link
Member

Choose a reason for hiding this comment

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

What benefit do we get from defining methods in protocol extensions instead of generating them directly? It seems that if we generated these methods, then we wouldn't need to expose the "Internal" versions in the protocols.

Would it be idiomatic Swift to use an underscore prefix instead of "Internal" in the helper method names? e.g. "_receive" instead of "receiveInternal"? The leading underscore seems like a more widely-understood indicator that something is private.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As explained before, methods declared in protocols can’t have default arguments, while methods declared in protocol extensions can. So we need to have a method in the extension unless we want users to provide all method arguments for each call.

Good point about the underscores, will look into that.

@MrMage
Copy link
Collaborator Author

MrMage commented Apr 16, 2018

Friendly ping :-)

@MrMage
Copy link
Collaborator Author

MrMage commented Apr 17, 2018

Replaced "{send,receive}Internal" with "_{send,receive}". Please take another look.

@timburks timburks merged commit 1a9f7e2 into grpc:master Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants