-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Allow fail messages to be caught, introduce Any trait #9967
Conversation
cc @brson and @catamorphism |
FailCauseAny(~Any), | ||
|
||
/// Failed because of linked failure | ||
FailCauseLinked |
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.
Can linked failure bubble up the failure cause? Presumably this is well-defined, since every task has exactly one parent, so ownership of the FailCause
can be transferred to 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.
The way it is currently implemented, it's actually more like N tasks can cause M tasks to fail due to linked failure. I attempted this at first, but it lead me down a rabbit hole of having a linked list of atomically reference counted fail causes, just so that every tasks that listens to task failure can receive them. :)
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.
Ah, can you link two arbitrary tasks for linked failure? (Not just parent/child tasks?)
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.
I think the hierarchy is still a tree, but a failure notifies all ancestors, which in combination with having multiple children that might all have failed leads to a directed graph of failure propagations. :)
At the moment this is not a problem because that part of the task unwinding system uses a boolean success
flag, which means all the various combinations just lead to a few AND
s and OR
s, but a lot of refactoring would be necessary if you wanted to use a sendable type there.
We should probably discuss |
Looks good! I think that I too would like tasks to just always fail with an In the future we will want to consider whether this should actually be a special dynamic |
(Needs a rebase.) |
So, how should I proceed? In case of changing this PR to only use On the
|
cc me |
After looking at it more deeply, for the case where the
|
I'd prefer not to expose The issue of what happens on OOM is academic at this point since what actually happens is the runtime aborts. Implementation details can be finessed when the time comes. |
I found an easier solution for returning the Instead of allocating the For this, I implemented a |
I updated this PR to return a SummaryThis PR allows the cause of a failure to be received in a task's future result and as the Task failure
Any
Other changes
|
I would rather |
@brson: Hm, good catch about the About |
After the last rebase,
I'm not sure how to deal with that/what caused it. |
Some code cleanup, sorting of import blocks Removed std::unstable::UnsafeArc's use of Either Added run-fail tests for the new FailWithCause impls Changed future_result and try to return Result<(), ~Any>. - Internally, there is an enum of possible fail messages passend around. - In case of linked failure or a string message, the ~Any gets lazyly allocated in future_results recv method. - For that, future result now returns a wrapper around a Port. - Moved and renamed task::TaskResult into rt::task::UnwindResult and made it an internal enum. - Introduced a replacement typedef `type TaskResult = Result<(), ~Any>`.
# Summary This PR allows the cause of a failure to be received in a task's future result and as the `Err` case of `task::try`, and also implements dynamic typing in form of an `Any` trait. # Task failure - `fail!` and related macros now accept 5 kinds of types: `~str`, `&'static str`, `std::send_str::SendStr`, `~std::any::Any` and `~T` where `T: Any + Send + 'static` - `std::task::TaskResult` got transformed into an internal enum `std::rt::task::UnwindResult`, and it's `Failure` variant now contains a value of enum type `UnwindReason`: - `UnwindReasonStr(SendStr)` maps to failing with a value of type `~str`, `&'static str` or `SendStr`. - `UnwindReasonAny(~Any)` maps to failing with an `~Any` or `~T` with `T: Send + 'static`. - `UnwindReasonLinked` maps to failing because the task got killed by linked failure. - Instead, `std::task::TaskResult` is now a typedef for `Result<(), ~Any>`, and both `TaskBuilder`'s `future_result()` and `task::try` now work with a value of this type. - `future_result()` no longer returns a `Port<TaskResult>`, instead it returns a wrapper `TaskResultPort` that implements `GenericPort` and `Peekable`, and which lazily allocates a `~` box and casts to `~Any` in case of failure with a `SendStr` or linked failure (for the latter case a unit struct `LinkedFailure` got added.) - Because `fail!` collapses both `~str` and `&'static str` into a `SendStr`, checking if a task error value is a string will just require a `.is::<SendStr>()` check, with `~str` and `&'static str` only being possible in case of an explicit `fail!(~~"...")` or `fail!(~ (&"...") as ~Any)`: # Any - In order to allow failing with arbitrary data, the `Any` trait got implemented. - It is being used in form of a trait object, usually `~Any` or `&Any`. - `&Any`, `~Any` and `&mut Any` have a few extension methods implemented on them: - `is<T>(self) -> bool` returns true if the `Any` object contains type `T` - `as_ref<T>(self) -> Option<&T>` returns a reference to the contained type, if it is `T` - `as_mut<T>(self) -> Option<&mut T>` returns a mutable reference to the contained type, if it is `T` - `move<T>(self) -> Option<~T>` allows to get the `~T` out of an `~Any` again. - `Any` currently uses type descriptors as a type id for comparisons, which is - not reliable, as it is not guaranteed that every type has only one type descriptor. - But safe, as no two types share the same type descriptor. - The implementation also a few `transmute`s, mostly to cast a `*Void` of the wrapped type into it's actual type `&T`, `&mut T` or `~T`. # Other changes - `std::unstable::UnsafeArc::try_unwrap` no longer uses `Either`, bringing us one step closer to removing that type. - A few of the touched modules got their import lines sorted. - A few stylistic cleanups here and there.
I'm sorry if this has already been brought up, but could |
@blake2-ppc Hm, good point. |
why are we trying to remove the Either type? the point of it is to allow users to avoid open-coding short sum types when they don't care about naming the arms - are we trying to discourage this practice? removing it incurs a bunch of boilerplate, as exhibited here. |
@bblum We've discussed the Either type in a team meeting before. The feeling among the core team is, as far as I can tell, that (like boolean return values) Either is a bad code smell. Some folks even suggested removing the type from the standard library. I'm not sure I'd go that far, but I agree that it's confusing to use Either when it would be almost as easy to write a one-off data type that provides better documentation. I don't expect this decision to be revisited. |
Remove blank lines when needless_return returns no value fix rust-lang/rust-clippy#9416 changelog: [`needless_return`] improve result format r? `@llogiq`
Summary
This PR allows the cause of a failure to be received in a task's future result and as the
Err
case oftask::try
, and also implements dynamic typing in form of anAny
trait.Task failure
fail!
and related macros now accept 5 kinds of types:~str
,&'static str
,std::send_str::SendStr
,~std::any::Any
and~T
whereT: Any + Send + 'static
std::task::TaskResult
got transformed into an internal enumstd::rt::task::UnwindResult
, and it'sFailure
variant now contains a value of enum typeUnwindReason
:UnwindReasonStr(SendStr)
maps to failing with a value of type~str
,&'static str
orSendStr
.UnwindReasonAny(~Any)
maps to failing with an~Any
or~T
withT: Send + 'static
.UnwindReasonLinked
maps to failing because the task got killed by linked failure.std::task::TaskResult
is now a typedef forResult<(), ~Any>
, and bothTaskBuilder
'sfuture_result()
andtask::try
now work with a value of this type.future_result()
no longer returns aPort<TaskResult>
, instead it returns a wrapperTaskResultPort
that implementsGenericPort
andPeekable
, and which lazily allocates a~
box and casts to~Any
in case of failure with aSendStr
or linked failure (for the latter case a unit structLinkedFailure
got added.)fail!
collapses both~str
and&'static str
into aSendStr
, checking if a task error value is a string will just require a.is::<SendStr>()
check, with~str
and&'static str
only being possible in case of an explicitfail!(~~"...")
orfail!(~ (&"...") as ~Any)
:Any
Any
trait got implemented.~Any
or&Any
.&Any
,~Any
and&mut Any
have a few extension methods implemented on them:is<T>(self) -> bool
returns true if theAny
object contains typeT
as_ref<T>(self) -> Option<&T>
returns a reference to the contained type, if it isT
as_mut<T>(self) -> Option<&mut T>
returns a mutable reference to the contained type, if it isT
move<T>(self) -> Option<~T>
allows to get the~T
out of an~Any
again.Any
currently uses type descriptors as a type id for comparisons, which istransmute
s, mostly to cast a*Void
of the wrapped type into it's actual type&T
,&mut T
or~T
.Other changes
std::unstable::UnsafeArc::try_unwrap
no longer usesEither
, bringing us one step closer to removing that type.