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

Removed statements with side effects from asserts #402

Merged
merged 1 commit into from
Jul 3, 2017
Merged

Conversation

antoninbas
Copy link
Member

In release builds, it is common to define NDEBUG to ignore
asserts. Which means it is a bad idea to call methods with necessary
side-effects inside asserts...

@codecov-io
Copy link

codecov-io commented Jul 1, 2017

Codecov Report

Merging #402 into master will decrease coverage by 0.06%.
The diff coverage is 37.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
- Coverage   76.55%   76.49%   -0.07%     
==========================================
  Files         114      115       +1     
  Lines        8977     8992      +15     
==========================================
+ Hits         6872     6878       +6     
- Misses       2105     2114       +9
Impacted Files Coverage Δ
src/bm_sim/match_tables.cpp 77.45% <ø> (ø) ⬆️
include/bm/bm_sim/nn.h 63.26% <ø> (ø) ⬆️
src/bm_sim/dev_mgr_bmi.cpp 0% <0%> (ø) ⬆️
include/bm/bm_sim/actions.h 31.42% <0%> (ø) ⬆️
src/bm_sim/match_units.cpp 90.21% <0%> (-0.18%) ⬇️
src/bm_sim/debugger.cpp 2% <0%> (+0.01%) ⬆️
src/bm_sim/_assert.cpp 0% <0%> (ø)
src/bm_sim/parser_error.cpp 97.61% <100%> (ø) ⬆️
src/bm_sim/simple_pre.cpp 59.09% <100%> (+0.47%) ⬆️
src/bm_sim/lookup_structures.cpp 100% <100%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c374a3...12d23f7. Read the comment docs.

In release builds, it is common to define NDEBUG to ignore
asserts. Which means it is a bad idea to call methods with necessary
side-effects inside asserts...
@antoninbas antoninbas merged commit 1d17819 into master Jul 3, 2017
@antoninbas antoninbas deleted the fix-asserts branch July 3, 2017 17:55
antoninbas added a commit that referenced this pull request Jul 3, 2017
In release builds, it is common to define NDEBUG to ignore
asserts. Which means it is a bad idea to call methods with necessary
side-effects inside asserts...
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.

2 participants