-
Notifications
You must be signed in to change notification settings - Fork 944
Fix stale code in Ensure #6997
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
Fix stale code in Ensure #6997
Conversation
Also make Ensure use all its argument even in debug mode, so that these things are easier to notice.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6997 +/- ##
==========================================
+ Coverage 80.06% 81.76% +1.69%
==========================================
Files 190 199 +9
Lines 37181 37013 -168
Branches 9450 9671 +221
==========================================
+ Hits 29770 30262 +492
+ Misses 2997 2864 -133
+ Partials 4414 3887 -527 ☔ View full report in Codecov by Sentry. |
#define Ensure(COND, FMT, ...) \ | ||
do \ | ||
{ \ | ||
if (unlikely(!(COND))) \ | ||
{ \ | ||
Assert(false); \ |
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.
Is this correct? I mean if the condition is false it is intended to ereport on release builds. Or am I missing something?
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.
Assert(false) will only be active in debug builds and trigger coredump, on prod builds you will get the error
Also make Ensure use all its argument even in debug mode, so that these things are easier to notice.
The problem was introduced by #6838
Disable-check: force-changelog-file