Thread local try with #43158
Thread local try with #43158
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
src/libstd/io/stdio.rs
Outdated
| @@ -674,20 +674,14 @@ fn print_to<T>(args: fmt::Arguments, | |||
| local_s: &'static LocalKey<RefCell<Option<Box<Write+Send>>>>, | |||
| global_s: fn() -> T, | |||
| label: &str) where T: Write { | |||
| let result = match local_s.state() { | |||
| LocalKeyState::Uninitialized | | |||
There was a problem hiding this comment.
This isn't quite the same behavior as before because Uninitialized would route to the global logger whereas this implementation would lazily run initialization and then route to the global logger.
| #[unstable(feature = "thread_local_state", | ||
| reason = "state querying was recently added", | ||
| issue = "27716")] | ||
| pub struct AccessError { |
There was a problem hiding this comment.
Can you reexport this outside the local module?
There was a problem hiding this comment.
Done, but comment not marked as outdated.
|
|
||
| impl Drop for Foo { | ||
| fn drop(&mut self) { | ||
| assert!(FOO.try_with(|_| panic!("`try_with` closure run")).is_err()); |
There was a problem hiding this comment.
Could you add an assertion that this code is run as well?
There was a problem hiding this comment.
Done, but comment not marked as outdated.
arielb1
added
the
S-waiting-on-review
|
Thanks! As one final comment, can you reimplement |
|
@bors: r+ |
|
📌 Commit a301f84 has been approved by |
|
⌛ Testing commit a301f84 with merge 98a0110fcc1386ff85373c741108b4967c5cd879... |
|
@bors retry -- This won't pass CI anyway due to android disk problems, prioritize the fix |
|
⌛ Testing commit a301f84 with merge a9859cbe80b561a1334e1e80f681f06673820b79... |
|
💔 Test failed - status-travis |
|
@bors: retry
…On Wed, Jul 12, 2017 at 6:52 PM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/253014173?utm_source=github_status&utm_medium=notification>
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#43158 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95Pivirl9c1TBPzcLfbsb0QSM44JZks5sNVxSgaJpZM4OTku8>
.
|
Thread local try with rust-lang/rfcs#2030 was turned into this PR (the RFC was closed, but it looks like just a PR should be good). See also: state stabilization issue: #27716 `try_with` is used in two places in std: stdio and thread_info. In stdio, it would be better if the result was passed to the closure, but in thread_info, it's better as is where the result is returned from the function call. I'm not sure which is better, but I prefer the current way as it better represents the scope.
|
☀️ Test successful - status-appveyor, status-travis |
rust-lang/rfcs#2030 was turned into this PR (the RFC was closed, but it looks like just a PR should be good).
See also: state stabilization issue: #27716
try_withis used in two places in std: stdio and thread_info. In stdio, it would be better if the result was passed to the closure, but in thread_info, it's better as is where the result is returned from the function call. I'm not sure which is better, but I prefer the current way as it better represents the scope.