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

An attribute to indicate struct size fields #433

Closed
kennykerr opened this issue Apr 23, 2021 · 7 comments · Fixed by #1479
Closed

An attribute to indicate struct size fields #433

kennykerr opened this issue Apr 23, 2021 · 7 comments · Fixed by #1479
Assignees
Labels
rust Critical for Rust adoption usability Touch-up to improve the user experience for a language projection

Comments

@kennykerr
Copy link
Contributor

Many structs like WNDCLASSEXA have a size field that should be initialized with the size of the struct. If there were an attribute on such fields, language projections could automatically initialize this field.

First reported here: microsoft/windows-rs#722

@mikebattista mikebattista added the usability Touch-up to improve the user experience for a language projection label Apr 23, 2021
@AArnott
Copy link
Member

AArnott commented Apr 26, 2021

Also requested here: microsoft/CsWin32#108

@mikebattista
Copy link
Contributor

If we do anything here, we could detect structs with cbSize and automatically add the attribute. We could then fix up the emitter manually for other cases.

@KalleOlaviNiemitalo
Copy link

How would the attribute work with STARTUPINFOEXW? That has a member STARTUPINFOW StartupInfo, which has a member DWORD cb; but StartupInfo.cb must be sizeof(STARTUPINFOEXW) rather than sizeof(STARTUPINFOW).

SP_PROPCHANGE_PARAMS likewise has a member SP_CLASSINSTALL_HEADER ClassInstallHeader, which has a member DWORD cbSize; but I'm not sure whether that one has to be sizeof(SP_CLASSINSTALL_HEADER) or sizeof(SP_PROPCHANGE_PARAMS).

@kennykerr
Copy link
Contributor Author

We could place the attribute on the struct rather than on the field with the path to the size field. Perhaps something like:

[StructSizeField("StartupInfo.cb")]
struct STARTUPINFOEXW { ... }

@KalleOlaviNiemitalo
Copy link

Alternatively, the attribute could be on both STARTUPINFOEXW::StartupInfo and STARTUPINFOW::cb. So it would mean "the size is somewhere in this field" rather than "the size is the value of this field".

@mikebattista
Copy link
Contributor

Is this instead of trying to automatically tag things with cbSize fields? We would add these attributes manually?

@AArnott
Copy link
Member

AArnott commented Sep 28, 2022

I think @KalleOlaviNiemitalo is pointing out odd cases where the field isn't even on the struct itself. The assumption I was making when suggesting we just look at field name to get us 80% of the way there was that the field would be on the same struct it describes. But in the cases called out above, that isn't necessarily the case, so whatever mechanism the metadata may use to describe these fields should ideally be able to describe these corner cases, such that even if they have to be specially described in an .rsp file in this repo, the projections could then do the Right ThingTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Critical for Rust adoption usability Touch-up to improve the user experience for a language projection
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants