-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
macros: add "local" runtime flavor #7375
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
base: master
Are you sure you want to change the base?
Changes from all commits
6bd6650
dad546f
225f6d7
11f49dd
8bfcd4b
324222a
0bdde71
92e156f
3a29832
213598d
0cc0f31
b58144e
e1117ec
8c3d838
0f436ec
67abbc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,15 @@ type AttributeArgs = syn::punctuated::Punctuated<syn::Meta, syn::Token![,]>; | |
enum RuntimeFlavor { | ||
CurrentThread, | ||
Threaded, | ||
Local, | ||
} | ||
|
||
impl RuntimeFlavor { | ||
fn from_str(s: &str) -> Result<RuntimeFlavor, String> { | ||
match s { | ||
"current_thread" => Ok(RuntimeFlavor::CurrentThread), | ||
"multi_thread" => Ok(RuntimeFlavor::Threaded), | ||
"local" => Ok(RuntimeFlavor::Local), | ||
"single_thread" => Err("The single threaded runtime flavor is called `current_thread`.".to_string()), | ||
"basic_scheduler" => Err("The `basic_scheduler` runtime flavor has been renamed to `current_thread`.".to_string()), | ||
"threaded_scheduler" => Err("The `threaded_scheduler` runtime flavor has been renamed to `multi_thread`.".to_string()), | ||
|
@@ -177,15 +179,16 @@ impl Configuration { | |
use RuntimeFlavor as F; | ||
|
||
let flavor = self.flavor.unwrap_or(self.default_flavor); | ||
|
||
let worker_threads = match (flavor, self.worker_threads) { | ||
(F::CurrentThread, Some((_, worker_threads_span))) => { | ||
(F::CurrentThread | F::Local, Some((_, worker_threads_span))) => { | ||
let msg = format!( | ||
"The `worker_threads` option requires the `multi_thread` runtime flavor. Use `#[{}(flavor = \"multi_thread\")]`", | ||
self.macro_name(), | ||
); | ||
return Err(syn::Error::new(worker_threads_span, msg)); | ||
} | ||
(F::CurrentThread, None) => None, | ||
(F::CurrentThread | F::Local, None) => None, | ||
(F::Threaded, worker_threads) if self.rt_multi_thread_available => { | ||
worker_threads.map(|(val, _span)| val) | ||
} | ||
|
@@ -207,7 +210,7 @@ impl Configuration { | |
); | ||
return Err(syn::Error::new(start_paused_span, msg)); | ||
} | ||
(F::CurrentThread, Some((start_paused, _))) => Some(start_paused), | ||
(F::CurrentThread | F::Local, Some((start_paused, _))) => Some(start_paused), | ||
(_, None) => None, | ||
}; | ||
|
||
|
@@ -219,7 +222,7 @@ impl Configuration { | |
); | ||
return Err(syn::Error::new(unhandled_panic_span, msg)); | ||
} | ||
(F::CurrentThread, Some((unhandled_panic, _))) => Some(unhandled_panic), | ||
(F::CurrentThread | F::Local, Some((unhandled_panic, _))) => Some(unhandled_panic), | ||
(_, None) => None, | ||
}; | ||
|
||
|
@@ -408,13 +411,30 @@ fn parse_knobs(mut input: ItemFn, is_test: bool, config: FinalConfig) -> TokenSt | |
.unwrap_or_else(|| Ident::new("tokio", last_stmt_start_span).into_token_stream()); | ||
|
||
let mut rt = match config.flavor { | ||
RuntimeFlavor::CurrentThread => quote_spanned! {last_stmt_start_span=> | ||
#crate_path::runtime::Builder::new_current_thread() | ||
}, | ||
RuntimeFlavor::CurrentThread | RuntimeFlavor::Local => { | ||
quote_spanned! {last_stmt_start_span=> | ||
#crate_path::runtime::Builder::new_current_thread() | ||
} | ||
} | ||
RuntimeFlavor::Threaded => quote_spanned! {last_stmt_start_span=> | ||
#crate_path::runtime::Builder::new_multi_thread() | ||
}, | ||
}; | ||
|
||
let mut checks = vec![]; | ||
let mut attrs = vec![]; | ||
|
||
let build = if let RuntimeFlavor::Local = config.flavor { | ||
checks.push(quote! { | ||
#[cfg(not(tokio_unstable))] | ||
compile_error!("The local runtime flavor is only available when `tokio_unstable` is set."); | ||
}); | ||
attrs.push(quote! { #[cfg(tokio_unstable)] }); | ||
quote_spanned! {last_stmt_start_span=> build_local(Default::default())} | ||
} else { | ||
quote_spanned! {last_stmt_start_span=> build()} | ||
}; | ||
|
||
if let Some(v) = config.worker_threads { | ||
rt = quote_spanned! {last_stmt_start_span=> #rt.worker_threads(#v) }; | ||
} | ||
|
@@ -437,14 +457,21 @@ fn parse_knobs(mut input: ItemFn, is_test: bool, config: FinalConfig) -> TokenSt | |
let body_ident = quote! { body }; | ||
// This explicit `return` is intentional. See tokio-rs/tokio#4636 | ||
let last_block = quote_spanned! {last_stmt_end_span=> | ||
#(#checks)* | ||
#(#attrs)* | ||
#[allow(clippy::expect_used, clippy::diverging_sub_expression, clippy::needless_return)] | ||
{ | ||
return #rt | ||
.enable_all() | ||
.build() | ||
.#build | ||
.expect("Failed building the Runtime") | ||
.block_on(#body_ident); | ||
} | ||
|
||
#[allow(unreachable_code)] | ||
{ | ||
panic!("fell through checks") | ||
} | ||
Comment on lines
+471
to
+474
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code needed? (since this is after the return statement, I think it will never be executed.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or, does this work as a safeguard for future macro changes? |
||
}; | ||
|
||
let body = input.body(); | ||
|
Twey marked this conversation as resolved.
Show resolved
Hide resolved
|
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.
Please add logic to
rt_multi_thread_available
to detect whether--cfg tokio_unstable
is set and emit a hard error if it is not.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'm not sure I understand this comment. Did you mean to add the logic to
RuntimeFlavor::from_str
?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 error should be similar to this error:
tokio/tokio-macros/src/entry.rs
Lines 193 to 197 in 2506c9f
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.
Maybe you meant a0ca77e; if I've misinterpreted please feel free to suggest changes :)
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.
Sorry, I just saw your comment. Is 4363203 more what you had in mind?
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 both are okay.
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.
This is harder than it appears because
RUSTFLAGS
is not passed to proc macros. I guess we have to emit acfg
check, so it actually can't live in either of these places.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.
44a31b1 seems to work in my local tests.
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.
Doesn't that also trigger other unrelated errors when it fails?
What we do today is essentially define the
main
macro two times to pass different values to thert_multi_thread
boolean, and havelib.rs
do this:This way, the macro knows based on the cfgs that apply in the main Tokio crate. We could use the same approach and have four macros.
Uh oh!
There was an error while loading. Please reload this page.
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 quadratic explosion is a little bit scary to me. We can avoid emitting additional code if checks fail like 213598d.