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

[Windows] Mark old_report_mode as unused #1046

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RandomInEqualities
Copy link
Contributor

When compiling in release mode the _CrtSetReportMode macro gets replaced by void(0). This makes old_report_mode unused, which visual studio warns about.

@busterb
Copy link
Contributor

busterb commented Apr 9, 2024

Could you add a comment in the code for reference later about why this is needed? Thanks!

Comment on lines 166 to 171
#define BEGIN_SUPPRESS_IPH \
int old_report_mode = _CrtSetReportMode(_CRT_ASSERT, 0); \
(void)old_report_mode; \
_invalid_parameter_handler old_handler = _set_thread_local_invalid_parameter_handler(noop_handler)
#define END_SUPPRESS_IPH \
_CrtSetReportMode(_CRT_ASSERT, old_report_mode); \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#define BEGIN_SUPPRESS_IPH \
int old_report_mode = _CrtSetReportMode(_CRT_ASSERT, 0); \
(void)old_report_mode; \
_invalid_parameter_handler old_handler = _set_thread_local_invalid_parameter_handler(noop_handler)
#define END_SUPPRESS_IPH \
_CrtSetReportMode(_CRT_ASSERT, old_report_mode); \
#define BEGIN_SUPPRESS_IPH \
int old_report_mode = _CrtSetReportMode(_CRT_ASSERT, 0); \
_invalid_parameter_handler old_handler = _set_thread_local_invalid_parameter_handler(noop_handler)
#define END_SUPPRESS_IPH \
(void)old_report_mode; \
_CrtSetReportMode(_CRT_ASSERT, old_report_mode); \

Wouldn't it be better to move the warning suppression into END_SUPPRESS_IPH? This way such warning would still correctly signal when missing an END_SUPPRESS_IPH by accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Year I think that sounds good :)

When compiling in release mode the _CrtSetReportMode macro
gets replaced by void(0). This makes old_report_mode unused.
Silence a warning about it.
@RandomInEqualities
Copy link
Contributor Author

I updated it and added const to the variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants