Compile examples and spirv-std on stable #340
Conversation
VZout
requested review from
khyperia
and removed request for
fu5ha and
khyperia
| #[spirv(block)] | ||
| #[derive(Copy, Clone)] | ||
| #[allow(unused_attributes)] | ||
| #[spirv(block)] |
There was a problem hiding this comment.
any reason for this, or just formatting?
There was a problem hiding this comment.
You can't put proc macros bellow derives.
| let end = item_str[start..item_str.len()].find(")]"); | ||
| item_str.replace_range(start..start + end.unwrap() + 2, ""); | ||
| } | ||
| TokenStream::from_str(item_str.as_str()).unwrap() |
There was a problem hiding this comment.
the quadratic reparse time here makes me sad, but, I'll leave this up to @eddyb if they think this is OK since they know more about proc macros than me
There was a problem hiding this comment.
"quadratic reparse time" makes me sound really smart. 👍
| while let Some(start) = item_str.find("#[spirv(") { | ||
| let end = item_str[start..item_str.len()].find(")]"); | ||
| item_str.replace_range(start..start + end.unwrap() + 2, ""); | ||
| } | ||
| TokenStream::from_str(item_str.as_str()).unwrap() |
There was a problem hiding this comment.
Can you implement this by looking for [...] groups instead (preceded by #), in the TokenStream? Using strings feels very error-prone.
Ideally you could/would use syn but that seems like it would be a lot more work.
| use proc_macro::TokenStream; | ||
| use std::str::FromStr; | ||
|
|
||
| #[cfg(not(target_arch = "spirv"))] |
There was a problem hiding this comment.
This won't do anything, I don't think, Cargo will only ever compile this for the host.
There was a problem hiding this comment.
I think it did compile it unless I put the crate behind a [target.dependency] thing
There was a problem hiding this comment.
Yep your right doesn't get compiled.
| @@ -0,0 +1,16 @@ | |||
| #![cfg_attr(target_arch = "spirv", feature(register_attr), register_attr(spirv))] | |||
There was a problem hiding this comment.
I don't think this does anything?
There was a problem hiding this comment.
Why not? It only registers the spriv attribute on spirv architecture.
spirv-attribThis would be a huge step forward for ark integration.