Skip to content
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

Slices should use default (optional) type #2293

Merged
merged 3 commits into from Jan 18, 2023
Merged

Slices should use default (optional) type #2293

merged 3 commits into from Jan 18, 2023

Conversation

kennykerr
Copy link
Collaborator

This effectively reverts #2233 - it turns out that some DirectX APIs require arrays of pointers where some or all must be null.

Fixes: #2286

Copy link
Member

@damyanp damyanp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that it's painful to force Optional on all arrays here. I guess the proper way to do this would be to get some metadata to describe that the array elements themselves are optional (I don't think that the SAL annotation in the D3D headers cover this already, so it'd be a game of whack-a-mole to try and find all the right places to add it).

@kennykerr
Copy link
Collaborator Author

Yep, I don't see a good way to do this retroactively.

@ChrisDenton
Copy link
Collaborator

Isn't a recurring problem that DirectX APIs are COM-like rather than actual COM? I mean they look like COM but can differ in various ways. If so then I'm of the tentative opinion that DirectX APIs should be handled differently from normal Windows API COM.

@kennykerr
Copy link
Collaborator Author

kennykerr commented Jan 18, 2023

The windows crate does not distinguish, nor care, whether a given API depends on the COM runtime services. It only cares that a given API inherits from IUnknown and thus supports dynamic discovery and lifetime management. That is why it can transparently support both WinRT and IUnknown-based APIs as they share this in common. Short of additional metadata, I believe the change here is correct however inconvenient it may be for some APIs.

Of course, I'd encourage DirectX to introduce additional metadata that we can then light up in Rust. 😊

@damyanp
Copy link
Member

damyanp commented Jan 18, 2023

I'd encourage DirectX to introduce additional metadata that we can then light up in Rust. 😊

ID3D11DeviceContext's idl has pointer_default( unique ), which I think would be a good indicator that any pointer value in the interface could be NULL. I don't think that this attribute makes it through the .h -> .winmd translations.

DirectX APIs should be handled differently from normal Windows API COM.

I think we already have a bunch of general "do things differently" set in win32metadata for the d3d headers. Maybe this could be another one of them?

@kennykerr kennykerr merged commit e06dc97 into master Jan 18, 2023
@kennykerr kennykerr deleted the default-slices branch January 18, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants