-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Command line option --debugAssertBreak #1189
Conversation
strange win 32 bit test fails:
|
Clang mac builds fails with:
|
The clang issue is somehow a relationship issue. The plug-ins depends on header only assert.h. |
I think I have got a good solution. The different behavior is now moved to the logging class. |
src/util/assert.h
Outdated
#else | ||
qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); | ||
#endif | ||
qCritical("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Logging::kAssertPrefix?
Nice -- LGTM other than my one comment. |
Thank you for review! |
If you currently build with MIXXX_DEBUG_ASSERTIONS_FATAL, you cannot continue Mixxx after this.
But it should be not a problem because in a release build it will continue anyway.
The new command line option --debugAssertBreak can be used when Mixxx is started under a debugger. In case of a false debug assert, it just breaks. Without a debugger this is fatal.
Before, this PR I have tried to get this behavior by placing a breakpoint at qWarning inn assert.c but this slows down the debug process a lot, and it leads to false positives because of inlining and optimization.
This new option will also avoid to switch the MIXXX_DEBUG_ASSERTIONS_FATAL flag which leads to a time consuming full rebuild. Now the MIXXX_DEBUG_ASSERTIONS_FATAL is only respected if --debugAssertBreak is not passed.