Expanding on Root and a potential common-case #703
Conversation
| /// Be careful that you aren't short-circuiting with an early return before | ||
| /// your Root value gets into_inner'd or dropped. The following will cause | ||
| /// a runtime panic when your error case gets triggered: | ||
| /// ``` |
There was a problem hiding this comment.
This is failing doc tests for me locally. I'm not sure why it's not failing in CI. 😕
There was a problem hiding this comment.
I think I have fixed this, I wasn't even thinking about doc tests when I wrote the code.
src/handle/root.rs
Outdated
| @@ -43,6 +43,21 @@ impl<T: Object> Root<T> { | |||
| /// The caller _must_ ensure `Root::into_inner` or `Root::drop` is called | |||
| /// to properly dispose of the `Root<T>`. If the value is dropped without | |||
| /// calling one of these methods, it will *panic*. | |||
| /// | |||
| /// Be careful that you aren't short-circuiting with an early return before | |||
There was a problem hiding this comment.
Since this communicates the same as above, can it be reworded to present it as an example?
For example, early return with a
?may cause a function to return before freeing aRoot. ...
There was a problem hiding this comment.
I've attempted a rewording along the lines suggested here.
src/handle/root.rs
Outdated
| /// | ||
| /// Be careful that you aren't short-circuiting with an early return before | ||
| /// your Root value gets into_inner'd or dropped. The following will cause | ||
| /// a runtime panic when your error case gets triggered: |
There was a problem hiding this comment.
This is no longer strictly accurate after merging #700
There was a problem hiding this comment.
I've changed it to delineate panic prior to 0.8, slow path otherwise.
|
I merged the doc update with what I did here. |
Extracted from a real problem I had. I believe this is the only runtime error I encountered. Please let me know what you think.