Split up features. #6905
Split up features. #6905
Conversation
|
The diff that the entire thing has made is slightly confusing, I would recommend reading it separately (or at least |
…ures-main # Conflicts: # wgpu-types/src/lib.rs
# Conflicts: # wgpu-types/src/lib.rs
|
Would be interesting to run cargo-semver-checks against this change, might catch any subtleties missed |
Is that to catch changes to functions? If so it's probably better than my manual check. Edit: you would have to ignore all the things about bit changes as their size (and type) has changed. |
Yeah, just as a point of information, not a "blocker" kind of thing |
|
If you have it then could you run it? Otherwise, I'm fine to install it and see what it tells me. |
|
Semver-checks seems to think everything is fine. I also removed one of the functions out of the macro and it noticed so it's not just missed it. |
|
Due to all recent changes being docs I think this is ready for review. |
| /// Macro for creating sets of bitflags, we need this because there | ||
| /// are almost more flags than bits left and the number of flags is increasing. | ||
| macro_rules! bitflags_array { |
There was a problem hiding this comment.
[not really actionable]
initially it took a lot of convincing myself that it's needed to have practically everything inside this macro. After reading the code some more I can see that it's useful, but it pains me not even the struct definition itself is spelled out clearly outside of the macro :(
This is the only place where we need it and I'd rather swallow quite a bit of code duplication for easier to read code here, especially for new-comers. That said, not having to touch everything again if we end up adding a third subdivision is great 👍
There was a problem hiding this comment.
I'm mostly hoping new users will not need to read the macro, I have been trying to make sure all of the customisation is outside it.
wgpu-types/src/features.rs
Outdated
| #[cfg(feature = "serde")] | ||
| impl WriteHex for Bits { | ||
| fn write_hex<W: fmt::Write>(&self, mut writer: W) -> fmt::Result { | ||
| let [$($lower_inner_name,)*] = self.0; | ||
| $($lower_inner_name.write_hex(&mut writer)?;)* | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "serde")] | ||
| impl ParseHex for Bits { | ||
| #[allow(unused_assignments)] | ||
| fn parse_hex(input: &str) -> Result<Self, ParseError> { | ||
| let mut offset = 0; | ||
| $( | ||
| // A byte is two hex places - u8 (1 byte) = 0x00 (2 hex places). | ||
| let cur_input = &input[offset..(offset + (size_of::<$T>() * 2))]; | ||
| let $lower_inner_name = <$T>::from_str_radix(cur_input, 16).map_err(|_|ParseError::invalid_hex_flag(cur_input))?; | ||
| // Two hex chars is a byte. | ||
| offset += (size_of::<$T>() * 2); | ||
| )* | ||
| Ok(Self([$($lower_inner_name,)*])) | ||
| } | ||
| } |
There was a problem hiding this comment.
we should add some unit tests at least for more complex operations like these (and fmt etc.) since they're unlikely to be hit by examples and not entirely trivial
There was a problem hiding this comment.
That's a good idea, I've already had trouble with these.
There was a problem hiding this comment.
Having tests was a good idea, I've started on them and have now realised that they don't print zero value hex places before the start of the actual value.
|
one thing I forgot to do so far is to look at the rustdoc output: We need to make sure that this is all still sane and shows the right things. |
I could also not re-export wgpu/webgpu features structs to make users have to use |
Connections
Fixes #5247 as this can be infinitely extended (or at least until the stack is exhausted).
Description
Creates an extendable way to use sets of bitflags together. This then splits up features into two subpart: FeaturesWGPU and FeaturesWebGPU. These can be further extended or split if that is needed (e.g shader features vs other features). This is ready for review, tell me if this is the wrong path. I'm quite new to macros (this is probably my second), so tell me if I've missed a helpful feature.
Implementation
Internally this uses a struct as that was the easiest to interact with extendable macros (though externally this interacts with arrays). I think it wouldn't be too hard to switch everything to arrays, but I think it might be slightly messier.
Testing
All existing tests should cover this
Implemented
constFunctionsChecklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.