Basic types #1363
Basic types #1363
Conversation
In GStreamer's Vulkan bindings (`GstVulkan-1.0.gir`) we have a string
constant that starts and ends with a space. When the C program emits
the value of this constant through `printf()`, an erroneous `.trim()`
call here in Rust strips the trailing whitespace and causes the test for
this particular constant to fail:
---- cross_validate_constants_with_c stdout ----
Constant value mismatch for GST_VULKAN_SWAPPER_VIDEO_FORMATS
Rust: " { RGBA, BGRA, RGB, BGR } "
C: " { RGBA, BGRA, RGB, BGR }"
Address this by removing the `.trim()`, and simplifying some of the
iterator code away with a `.split_once()`.
All tests on at least GStreamer succeed, leading to believe that this
`.trim()` serves no purpose as the argument is already split on
newlines; any additional whitespace is hence part of the constant that
is being tested.
|
A next step would be to address the Vulkan specific hack. We need a cleaner way to trigger the "special" handling of Vulkan records: |
src/parser.rs
Outdated
| @@ -264,6 +265,13 @@ impl Library { | |||
| ) -> Result<(), String> { | |||
| if let Some(typ) = self.read_record(parser, ns_id, elem, None, None)? { | |||
| let name = typ.get_name(); | |||
| // TODO: Don't hardcode to "Vulkan" | |||
| let typ = if self.namespace(ns_id).name.eq("Vulkan") { | |||
| assert!(typ.is_incomplete(self)); | |||
There was a problem hiding this comment.
We probably won't keep this assert but it shows that Vulkan records are incomplete.
Unfortunately, non Vulkan records can be incomplete too so we can't use is_incomplete as the trigger for the new behavior.
And then there is the problem of the "ash::vk::{}" format string: where should it be specified if we want to externalize it (to make gir Vulkan agnostic again).
There was a problem hiding this comment.
An alternative approach to mapping the names with "ash::vk::{}" would be to not do it and, instead, generate rust type aliases in a post process step.
pub type vulkan::Bool32 = VkBool32;
Advantage is that the Vulkan namespace will not be erased anymore (less "magic").
Disadvantage is that a bit more rust code will be generated.
There was a problem hiding this comment.
Likewise, instead of tweaking the Vulkan namespace on the fly, we could introduce a postprocess hook that tweaks/transforms the namespace after it was parsed.
filnet
force-pushed
the
basic_types
branch
2 times, most recently
from
7746d63
to
e4bc4ca
Compare
|
I went with post processing the Vulkan namespace. |
also rename the new Basic::Vulkan enum value to Basic::Typedef
|
What is the status of this one? |
|
@bilelmoussaoui I don't think we ever continued the discussion how to best map |
This PR is a follow up of #1134. Not sure what is the accepted way of contributing to someone else's PR.
Changes include:
Basic::Vulkanenum toBasic::Typedef.Vulkan-1.0.gir currently contains only what can be considered as
typedefrecords.See https://gitlab.gnome.org/GNOME/gobject-introspection/-/blob/main/misc/update-vulkan-gir.py#L17
Basic::Typedefs when parsing theVulkannamespace.So a bit more generic and the
const VULKAN: &[&str] = &[...]is gone. :)