added MANIFOLD_ASSERT #1225
added MANIFOLD_ASSERT #1225
Conversation
| #include <iostream> | ||
| #include <string> | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Just a thought - should we be hiding the ReadOBJ/WriteOBJ methods behind an #ifdef MANIFOLD_DEBUG as well, since they use std::istream and std::ostream?
There was a problem hiding this comment.
Actually, nevermind - I'll fix that myself in a separate PR - should be orthogonal to this.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1225 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 31 31
Lines 5985 5987 +2
=======================================
+ Hits 5518 5520 +2
Misses 467 467 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -37,6 +37,7 @@ set( | |||
| # Primary user facing options | |||
| option(MANIFOLD_CROSS_SECTION "Build CrossSection for 2D support" ON) | |||
| option(MANIFOLD_DEBUG "Enable debug tracing/timing" OFF) | |||
| option(MANIFOLD_ASSERT "Enable assertions - requires MANIFOLD_DEBUG" OFF) | |||
There was a problem hiding this comment.
I remember there is a fancy dependent option thing that may be useful here. At least we may want to issue an error if the user only enabled MANIFOLD_ASSERT without enabling MANIFOLD_DEBUG?
There was a problem hiding this comment.
Good idea - want to throw that into a follow-up PR?
| @@ -1 +1,4 @@ | |||
| BasedOnStyle: Google | |||
| PointerAlignment: Left | |||
| ReferenceAlignment: Left | |||
| DerivePointerAlignment: false | |||
There was a problem hiding this comment.
I got tired of clang-format going back and forth on things between versions, and I think this'll help. I had to reformat the whole code base because apparently it was not being consistent about where the & was.
Fixes #1224
We incur about a 20% overhead for enabling assertions, which
MANIFOLD_DEBUGdid. Here I've addedMANIFOLD_ASSERTto control that explicitly.MANIFOLD_DEBUGcontrols the inclusion of all string and stream related headers, and I've now centralized those includes.