-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Add expression interpolation with #{expr}
#88
Conversation
One annoyance I frequently run into with the `quote!` macro is that I need to lift out all variable bindings that are being interpolated. For example I'll often do: let name = &foo.name; let abi = &foo.abi; quote! { extern #abi fn #name() { /* ... */ } } but I've recently had the idea that in addition to lighteight `#name` interpolation there's also available syntax (I believe) to support expression interpolation as well, implemented in this PR. The above snippet could now be rewritten as: quote! { extern #{foo.abi} fn #{foo.name} { /* ... */ } }
1796823
to
85f8a26
Compare
Ah thanks for the pointer, I was unaware of those! Reading over those I personally feel like the main use case for this is what was outlined here, accessing fields. I do that so often in so many quote impls that being able to do that would be quite beneficial. I will admit though that I haven't considered the repetition case at all here. I try to avoid using repetitions as they've always felt a little off to me running into issues like #7 and such. I would personally advocate for considering this. From my (likely naive) point of view this seems to be a simple enough addition that it is lightweight enough to add and is worth it. It sounds like @dtolnay like you're also worried about misuse causing hard-to-read code, and I definitely agree that's possible. I don't think that'd persuade me, though, to think this shouldn't be added on that basis. |
ping @dtolnay, do you have thoughts on this perhaps? |
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.
Thanks for the reminder. I continue to believe that we should not support this.
I do that so often in so many quote impls that being able to do that would be quite beneficial.
I know you have been working on wasm-bindgen so I found impl ToTokens for ast::StructField as one seemingly typical instance of this pattern.
//! current code
let name = &self.name;
let struct_name = &self.struct_name;
let ty = &self.ty;
let getter = &self.getter;
quote! {
#[no_mangle]
#[doc(hidden)]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
pub unsafe extern "C" fn #getter(js: u32)
-> <#ty as ::wasm_bindgen::convert::IntoWasmAbi>::Abi
{
use wasm_bindgen::__rt::{WasmRefCell, assert_not_null};
use wasm_bindgen::convert::{GlobalStack, IntoWasmAbi};
fn assert_copy<T: Copy>(){}
assert_copy::<#ty>();
let js = js as *mut WasmRefCell<#struct_name>;
assert_not_null(js);
let val = (*js).borrow().#name;
<#ty as IntoWasmAbi>::into_abi(
val,
&mut GlobalStack::new(),
)
}
}
It seems highly likely that you would have used #{self.var}
in this code if the feature existed. The thing is, I do not consider this an improvement. As a person reading your code, I would much prefer to read the original version.
//! proposed
quote! {
#[no_mangle]
#[doc(hidden)]
#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
pub unsafe extern "C" fn #{self.getter}(js: u32)
-> <#{self.ty} as ::wasm_bindgen::convert::IntoWasmAbi>::Abi
{
use wasm_bindgen::__rt::{WasmRefCell, assert_not_null};
use wasm_bindgen::convert::{GlobalStack, IntoWasmAbi};
fn assert_copy<T: Copy>(){}
assert_copy::<#{self.ty}>();
let js = js as *mut WasmRefCell<#{self.struct_name}>;
assert_not_null(js);
let val = (*js).borrow().#{self.name};
<#{self.ty} as IntoWasmAbi>::into_abi(
val,
&mut GlobalStack::new(),
)
}
}
If the local variables are named the same as fields of self
, like here, then skimming them does not hurt my ability to understand the code. The extra noise and punctuation and nesting and compile-time-vs-runtime dissonance inside the quote!
invocation does though.
And in cases where the local variable would be named differently from the field (like you have let import_name = &self.shim
later in the file), the new name gives an opportunity to attach context that makes the application of that field inside quote!
easier to understand.
Keep in mind that complexity inside of a quote!
invocation is far more expensive than a similar amount of complexity outside of an invocation.
That's all spot on!
This is an interesting point I think. I believe I was largely struct with inspiration from this when I saw the typed-html macro crop up recently and it used interpolation for Rust expressions, and I thought "oh right I would use that a lot for quoting!". I figured that many interpolation syntaxes support expression-based interpolation (like JS strings's Thinking more, though, this point about expressivity I think is key. In a string or in something like HTML the surrounding "stuff" is entirely different from what you're interpolating. With That all makes sense to me, and as I was unaware of the rejected proposals before I'm gonna go ahead and close this. Rationale makes sense to me! |
One annoyance I frequently run into with the
quote!
macro is that Ineed to lift out all variable bindings that are being interpolated. For
example I'll often do:
but I've recently had the idea that in addition to lighteight
#name
interpolation there's also available syntax (I believe) to support
expression interpolation as well, implemented in this PR. The above
snippet could now be rewritten as: