std: Make std::comm return types consistent

[alexcrichton]

There are currently a number of return values from the std::comm methods, not
all of which are necessarily completely expressive:

• Sender::try_send(t: T) -> bool
  This method currently doesn't transmit back the data t if the send fails
  due to the other end having disconnected. Additionally, this shares the name
  of the synchronous try_send method, but it differs in semantics in that it
  only has one failure case, not two (the buffer can never be full).
• SyncSender::try_send(t: T) -> TrySendResult<T>
  This method accurately conveys all possible information, but it uses a
  custom type to the std::comm module with no convenience methods on it.
  Additionally, if you want to inspect the result you're forced to import
  something from std::comm.
• SyncSender::send_opt(t: T) -> Option<T>
  This method uses Some(T) as an "error value" and None as a "success value",
  but almost all other uses of Option have Some/None the other way
• Receiver::try_recv(t: T) -> TryRecvResult<T>
  Similarly to the synchronous try_send, this custom return type is lacking in
  terms of usability (no convenience methods).

With this number of drawbacks in mind, I believed it was time to re-work the
return types of these methods. The new API for the comm module is:

Sender::send(t: T) -> ()
Sender::send_opt(t: T) -> Result<(), T>
SyncSender::send(t: T) -> ()
SyncSender::send_opt(t: T) -> Result<(), T>
SyncSender::try_send(t: T) -> Result<(), TrySendError<T>>
Receiver::recv() -> T
Receiver::recv_opt() -> Result<T, ()>
Receiver::try_recv() -> Result<T, TryRecvError>

The notable changes made are:

• Sender::try_send => Sender::send_opt. This renaming brings the semantics in
  line with the SyncSender::send_opt method. An asychronous send only has one
  failure case, unlike the synchronous try_send method which has two failure
  cases (full/disconnected).
• Sender::send_opt returns the data back to the caller if the send is guaranteed
  to fail. This method previously returned bool, but then it was unable to
  retrieve the data if the data was guaranteed to fail to send. There is still a
  race such that when Ok(()) is returned the data could still fail to be
  received, but that's inherent to an asynchronous channel.
• Result is now the basis of all return values. This not only adds lots of
  convenience methods to all return values for free, but it also means that you
  can inspect the return values with no extra imports (Ok/Err are in the
  prelude). Additionally, it's now self documenting when something failed or not
  because the return value has "Err" in the name.

Things I'm a little uneasy about:

• The methods send_opt and recv_opt are not returning options, but rather
  results. I felt more strongly that Option was the wrong return type than the
  _opt prefix was wrong, and I coudn't think of a much better name for these
  methods. One possible way to think about them is to read the _opt suffix as
  "optionally".
• Result<T, ()> is often better expressed as Option. This is only applicable
  to the recv_opt() method, but I thought it would be more consistent for
  everything to return Result rather than one method returning an Option.

Despite my two reasons to feel uneasy, I feel much better about the consistency
in return values at this point, and I think the only real open question is if
there's a better suffix for {send,recv}_opt.

Closes #11527

[liigo]

I'd suggest:

1. Remove send and rename send_opt to send.
2. Just rename send_opt to send_r, r means result/return.
   The same to recv.
   2014年4月11日 上午6:16于 "Alex Crichton" notifications@github.com写道：

There are currently a number of return values from the std::comm methods,
not
all of which are necessarily completely expressive:

Sender::try_send(t: T) -> bool
This method currently doesn't transmit back the data t if the send
fails
due to the other end having disconnected. Additionally, this shares
the name
of the synchronous try_send method, but it differs in semantics in
that it
only has one failure case, not two (the buffer can never be full).

SyncSender::try_send(t: T) -> TrySendResult
This method accurately conveys all possible information, but it uses a
custom type to the std::comm module with no convenience methods on it.
Additionally, if you want to inspect the result you're forced to import
something from std::comm.

SyncSender::send_opt(t: T) -> Option
This method uses Some(T) as an "error value" and None as a "success
value",
but almost all other uses of Option have Some/None the other way

Receiver::try_recv(t: T) -> TryRecvResult
Similarly to the synchronous try_send, this custom return type is
lacking in
terms of usability (no convenience methods).

With this number of drawbacks in mind, I believed it was time to re-work
the
return types of these methods. The new API for the comm module is:

Sender::send(t: T) -> ()
Sender::send_opt(t: T) -> Result<(), T>
SyncSender::send(t: T) -> ()
SyncSender::send_opt(t: T) -> Result<(), T>
SyncSender::try_send(t: T) -> Result<(), TrySendError>
Receiver::recv() -> T
Receiver::recv_opt() -> Result<T, ()>
Receiver::try_recv() -> Result<T, TryRecvError>

The notable changes made are:

Sender::try_send => Sender::send_opt. This renaming brings the
semantics in
line with the SyncSender::send_opt method. An asychronous send only
has one
failure case, unlike the synchronous try_send method which has two
failure
cases (full/disconnected).

Sender::send_opt returns the data back to the caller if the send is
guaranteed
to fail. This method previously returned bool, but then it was unable
to
retrieve the data if the data was guaranteed to fail to send. There is
still a
race such that when Ok(()) is returned the data could still fail to be
received, but that's inherent to an asynchronous channel.

Result is now the basis of all return values. This not only adds lots
of
convenience methods to all return values for free, but it also means
that you
can inspect the return values with no extra imports (Ok/Err are in the
prelude). Additionally, it's now self documenting when something
failed or not
because the return value has "Err" in the name.

Things I'm a little uneasy about:

The methods send_opt and recv_opt are not returning options, but rather
results. I felt more strongly that Option was the wrong return type
than the
_opt prefix was wrong, and I coudn't think of a much better name for
these
methods. One possible way to think about them is to read the _opt
suffix as
"optionally".

Result is often better expressed as Option. This is only applicable
to the recv_opt() method, but I thought it would be more consistent for
everything to return Result rather than one method returning an Option.

Despite my two reasons to feel uneasy, I feel much better about the
consistency
in return values at this point, and I think the only real open question is
if
there's a better suffix for {send,recv}_opt.

Closes #11527 #11527

You can merge this Pull Request by running

git pull https://github.com/alexcrichton/rust rework-chan-return-values

Or view, comment on, or merge it at:

#13448
Commit Summary

• std: Make std::comm return types consistent

File Changes

• M src/libgreen/sched.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-0(7)
• M src/libgreen/task.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-1(2)
• M src/libnative/io/timer_other.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-2(9)
• M src/libnative/io/timer_timerfd.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-3(9)
• M src/libnative/io/timer_win32.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-4(9)
• M src/libnative/task.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-5(2)
• M src/librustuv/net.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-6(2)
• M src/librustuv/signal.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-7(2)
• M src/librustuv/timer.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-8(12)
• M src/libstd/comm/mod.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-9(239)
• M src/libstd/comm/oneshot.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-10(14)
• M src/libstd/comm/select.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-11(18)
• M src/libstd/comm/shared.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-12(8)
• M src/libstd/comm/stream.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-13(24)
• M src/libstd/comm/sync.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-14(18)
• M src/libstd/io/comm_adapters.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-15(12)
• M src/libstd/io/net/udp.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-16(4)
• M src/libstd/io/signal.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-17(3)
• M src/libstd/io/timer.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-18(22)
• M src/libstd/rt/task.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-19(2)
• M src/libsync/comm.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-20(8)
• M src/libsync/lock.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-21(2)
• M src/libsync/raw.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-22(11)
• M src/test/bench/msgsend-pipes-shared.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-23(6)
• M src/test/bench/msgsend-pipes.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-24(6)
• M src/test/bench/task-perf-jargon-metal-smoke.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-25(2)
• M src/test/run-pass/issue-9396.rshttps://github.com/std: Make std::comm return types consistent #13448/files#diff-26(6)

Patch Links:

• https://github.com/mozilla/rust/pull/13448.patch
• https://github.com/mozilla/rust/pull/13448.diff

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/13448
.

[brson]

I'm pretty happy with this rationalization.

If we made send_opt the default send method then that forces every caller to check the result, which I've previously thought was too burdensome. For most protocols it's simply wrong for the receiver to just disappear so failing is a reasonable result.

[brson]

I r+ed this despite @liigo's comments since I think this is a good improvement. The suggested improvements look contentious enough to me that we can follow up on those topics later.

[brson]

@liigo Although the _r isn't currently a convention it's an interesting idea that I may be slowly warming to. I wonder why this case hasn't come up before though, and I wonder how many _opt variants we have these days - it may not be many.

[huonw]

One (weak) argument against _r is the meaning differs to the one in C, where it means "reentrant".

[liigo]

I like @alexcrichton's this improvement too. But for _opt names, ...
We already have Option, getopts::optopt, TaskOpts, a little mix up.