Skip to content

Improvements to HRESULT #666

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 7, 2022
Merged

Improvements to HRESULT #666

merged 6 commits into from
Sep 7, 2022

Conversation

@elachlan
Copy link
Contributor

@elachlan elachlan commented Aug 24, 2022 •
edited by AArnott
Loading

This overrides ToString() to produce the familiar 0x80001234 format for an HRESULT.
It also adds the HRESULT_FROM_WIN32 macro.

Fixes #664

@elachlan
Copy link
Contributor Author

I am unsure what to do to fix the build error. Do I need to specify a dependency somewhere?

AArnott
AArnott reviewed Aug 24, 2022
View reviewed changes
AArnott
AArnott reviewed Aug 24, 2022
View reviewed changes
@AArnott
Fix HRESULT_FROM_WIN32 definition to handle negative Win32 errors
45dd702 
Also ensure invariant formatting of the HRESULT
@AArnott
Remove HRESULT_FROM_WIN32 from HRESULT type
f53f248 
As a member of the HRESULT struct, it caused the otherwise small HRESULT struct to also generate the very large `WIN32_ERROR` enum, even if the user wouldn't have used this function.
So instead, we start a new concept where 'macros' such as this can be defined as a template, and then requested for generation explicitly. The generator is smart enough to detect what types the macro uses and generate them as well, as proven by the new test.
@AArnott
Merge remote-tracking branch 'origin/main' into HRESULT
7d21740
@AArnott
Copy link
Member

AArnott commented Sep 6, 2022

I refactored the HRESULT_FROM_WIN32 into its own optional 'macro' (a new concept) so that WIN32_ERROR doesn't always have to be (exhaustively) generated every time HRESULT is required.

@AArnott AArnott enabled auto-merge September 6, 2022 23:32
@AArnott AArnott added the enhancement New feature or request label Sep 6, 2022
@AArnott AArnott disabled auto-merge September 6, 2022 23:47
@AArnott AArnott enabled auto-merge September 6, 2022 23:49
@AArnott AArnott mentioned this pull request Sep 6, 2022
Closed
@AArnott AArnott merged commit ae4b609 into microsoft:main Sep 7, 2022
@elachlan elachlan deleted the HRESULT branch September 7, 2022 01:20
RussKie
RussKie reviewed Sep 7, 2022
View reviewed changes
RussKie
RussKie reviewed Sep 7, 2022
View reviewed changes
@dependabot dependabot bot mentioned this pull request Sep 15, 2025
Closed
@dependabot dependabot bot mentioned this pull request Oct 20, 2025
Closed
@dependabot dependabot bot mentioned this pull request Oct 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AArnott AArnott AArnott left review comments

+2 more reviewers

@RussKie RussKie RussKie left review comments

@JeremyKuhne JeremyKuhne JeremyKuhne left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improvements to HRESULT

4 participants

@elachlan @AArnott @RussKie @JeremyKuhne