Builder pattern allows invalid usage

[Timbals]

The generated builders zero out all fields other than ty when created. Some structures aren't valid with zeroed out fields, but they can be submitted to functions anyway. Some examples:

• XrCompositionLayerProjection::viewCount must be greater than 0.
• XrCompositionLayerProjection::space must be a valid XrSpace handle.
• XrInteractionProfileDpadBindingEXT::actionSet must be a valid XrActionSet handle
• generally everything that includes a handle

The solution to this is probably to take all required fields as arguments to the new functions instead of using a builder. I will work on a PR for this.

[Ralith]

I'm not sure this is worth pursuing. Large argument lists can significantly degrade readability compared to populating struct fields, and IIRC OpenXR guarantees graceful error detection for this case, so there is no safety issue. We can't statically prevent erroneous inputs in general, so we should be wary of worsening ergonomics in exchange for handling a few marginal cases.

[Timbals]

I think the spec doesn't guarantee graceful error detection:

Runtimes may detect XR_NULL_HANDLE and other invalid handles passed where a valid handle is required and return XR_ERROR_HANDLE_INVALID. However, runtimes are not required to do so unless otherwise specified, and so use of any invalid handle may result in undefined behavior.

But I agree that it may not be worth the extra complexity. Because all these requirements are basically that some field wasn't filled, the validation layer seems to catch them anyway.

[Ralith]

If UB is possible in a safe API, then we need to at least insert such checks ourselves, or the API is unsound.

[Timbals]

Adding asserts to all relevant functions seems like a good solution. It doesn't change the API.

Should I go through all structs/functions and create a PR?

[Ralith]

That would be very welcome! Note that handling this correctly for FrameStream::end in particular may be tricky as we don't statically know the input types. Might be easiest to just mark that one unsafe, or redesign its API somehow; perhaps a builder pattern for the call itself, with a generic method to add new layers that knows how to validate them?