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.

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

Improvements to HRESULT #666

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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeremyKuhne JeremyKuhne JeremyKuhne left review comments

@RussKie RussKie RussKie left review comments

@AArnott AArnott AArnott left review comments

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