This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Make set_windows_crt_report_mode.h more explicit
ClosedPublic

Authored by mstorsjo on Jul 20 2023, 5:35 AM.

Details

Summary

This header is included when building with a debug CRT in
MSVC/Clang-cl environments. By default, failed asserts with the
debug CRT pops up a blocking dialog box alerting the user about
the failed assert. When running more than one test in an automated
fashion, this isn't ideal.

This header tries to run initializers to set the behaviour of the
failed asserts to print a message to the console, just like the
default is in release mode.

This is previously done by setting the reporting mode to
_CRTDBG_MODE_DEBUG, which means outputting to the debugger's
output window. In some setups, this is enough for making it work,
but in others it instead can pop up a dialog asking for which
debugger to use.

Instead set the mode explicitly to _CRTDBG_MODE_FILE and set the
destination to be explicitly to stderr.

For setups where the previous code worked correctly, it doesn't make
any difference other than that a failed assert prints an additional
"abort() has been called" message that wasn't printed before.

Patch originally by Andrew Ng.

Diff Detail

Event Timeline

mstorsjo created this revision.Jul 20 2023, 5:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 5:35 AM
mstorsjo requested review of this revision.Jul 20 2023, 5:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 5:35 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I'm not too familiar with this. When others more familiar with Windows are happy with it I'm happy to approve for libc++.

LGTM, but then again I did author the originating patch, so perhaps someone else also needs to take a look?

LGTM, but then again I did author the originating patch, so perhaps someone else also needs to take a look?

Well I also looked at it, so that gives one author and one domain review, so I think that's quite enough.

Mordante accepted this revision as: Restricted Project.Jul 26 2023, 9:24 AM

LGTM, but then again I did author the originating patch, so perhaps someone else also needs to take a look?

Well I also looked at it, so that gives one author and one domain review, so I think that's quite enough.

Agreed, so I'm happy to approve for libc++.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 28 2023, 1:52 PM
This revision was automatically updated to reflect the committed changes.