Disable exceptions by default (#1044)

* removed ASSERT

* removed MANIFOLD_EXCEPTIONS

* fix CI

* fix performance regression

* avoid copying in Is2Manifold

* back to two assert flavors

---------

Co-authored-by: pca006132 <[email protected]>

.github/workflows/manifold.yml

@@ -108,9 +108,6 @@ jobs:
   build_wasm:
     timeout-minutes: 30
     runs-on: ubuntu-22.04
-    strategy:
-      matrix:
-        exception: [ON, OFF]
     steps:
     - name: Install dependencies
       run: |
@@ -130,7 +127,7 @@ jobs:
         source ./emsdk/emsdk_env.sh
         mkdir build
         cd build
-        emcmake cmake -DCMAKE_BUILD_TYPE=Release -DMANIFOLD_EXCEPTIONS=${{matrix.exception}} .. && emmake make
+        emcmake cmake -DCMAKE_BUILD_TYPE=Release .. && emmake make
     - name: Test WASM
       run: |
         cd build/test
@@ -144,7 +141,7 @@ jobs:
         cp ../manifold.* ./dist/
     - name: Upload WASM files
       uses: actions/upload-artifact@v4
-      if: matrix.exception == 'ON' && github.event_name == 'push'
+      if: github.event_name == 'push'
       with:
         name: wasm
         path: bindings/wasm/examples/dist/

.vscode/settings.json

@@ -128,7 +128,6 @@
     "-DMANIFOLD_PYBIND=ON",
     "-DMANIFOLD_EXPORT=ON",
     "-DMANIFOLD_DEBUG=ON",
-    "-DMANIFOLD_EXCEPTIONS=ON",
     "-DMANIFOLD_PAR=ON",
     "-DCODE_COVERAGE=OFF",
     "-DCMAKE_CXX_FLAGS=''" //'-fsanitize=address,undefined'"

CMakeLists.txt

@@ -53,7 +53,6 @@ option(
   "Allow Manifold build to download missing dependencies"
   ON
 )
-option(MANIFOLD_EXCEPTIONS "Build manifold with exception enabled" ON)
 option(MANIFOLD_EXPORT "Build mesh export (via assimp) utility library" OFF)
 option(MANIFOLD_PAR "Enable Parallel backend" OFF)
 option(
@@ -129,7 +128,7 @@ endif()
 
 if(EMSCRIPTEN)
   message("Building for Emscripten")
-  if(MANIFOLD_EXCEPTIONS)
+  if(MANIFOLD_DEBUG)
     list(APPEND MANIFOLD_FLAGS -fexceptions)
     string(
       APPEND
@@ -359,7 +358,6 @@ message(STATUS "MANIFOLD_DEBUG:              ${MANIFOLD_DEBUG}")
 message(STATUS "MANIFOLD_CBIND:              ${MANIFOLD_CBIND}")
 message(STATUS "MANIFOLD_PYBIND:             ${MANIFOLD_PYBIND}")
 message(STATUS "MANIFOLD_JSBIND:             ${MANIFOLD_JSBIND}")
-message(STATUS "MANIFOLD_EXCEPTIONS:         ${MANIFOLD_EXCEPTIONS}")
 message(STATUS "MANIFOLD_FLAGS:              ${MANIFOLD_FLAGS}")
 message(STATUS " ")
 

include/manifold/common.h

@@ -14,7 +14,6 @@
 
 #pragma once
 #include <limits>
-#include <stdexcept>
 #include <vector>
 
 #include "manifold/linalg.h"
@@ -561,24 +560,6 @@ class Quality {
 };
 /** @} */
 
-/** @addtogroup Exceptions
- *  @ingroup Optional
- *  @brief Custom Exceptions. Exceptions are only thrown if the
- * MANIFOLD_EXCEPTIONS flag is set.
- * @{
- */
-struct userErr : public virtual std::runtime_error {
-  using std::runtime_error::runtime_error;
-};
-struct topologyErr : public virtual std::runtime_error {
-  using std::runtime_error::runtime_error;
-};
-struct geometryErr : public virtual std::runtime_error {
-  using std::runtime_error::runtime_error;
-};
-using logicErr = std::logic_error;
-/** @} */
-
 /** @addtogroup Debug
  * @ingroup Optional
  * @{

include/manifold/optional_assert.h

@@ -14,34 +14,53 @@
 
 #pragma once
 
-#ifdef MANIFOLD_EXCEPTIONS
-#include <stdexcept>
-#endif
-
 #ifdef MANIFOLD_DEBUG
 #include <iostream>
 #include <sstream>
+#include <stdexcept>
 #include <string>
 
+/** @addtogroup Debug
+ * @{
+ */
+struct userErr : public virtual std::runtime_error {
+  using std::runtime_error::runtime_error;
+};
+struct topologyErr : public virtual std::runtime_error {
+  using std::runtime_error::runtime_error;
+};
+struct geometryErr : public virtual std::runtime_error {
+  using std::runtime_error::runtime_error;
+};
+using logicErr = std::logic_error;
+
 template <typename Ex>
-void Assert(bool condition, const char* file, int line, const std::string& cond,
-            const std::string& msg) {
-  if (!condition) {
-    std::ostringstream output;
-    output << "Error in file: " << file << " (" << line << "): \'" << cond
-           << "\' is false: " << msg;
-    throw Ex(output.str());
-  }
+void AssertFail(const char* file, int line, const char* cond, const char* msg) {
+  std::ostringstream output;
+  output << "Error in file: " << file << " (" << line << "): \'" << cond
+         << "\' is false: " << msg;
+  throw Ex(output.str());
 }
-#define DEBUG_ASSERT(condition, EX, msg) \
-  Assert<EX>(condition, __FILE__, __LINE__, #condition, msg);
-#else
-#define DEBUG_ASSERT(condition, EX, msg)
-#endif
 
-#ifdef MANIFOLD_EXCEPTIONS
+template <typename Ex>
+void AssertFail(const char* file, int line, const std::string& cond,
+                const std::string& msg) {
+  std::ostringstream output;
+  output << "Error in file: " << file << " (" << line << "): \'" << cond
+         << "\' is false: " << msg;
+  throw Ex(output.str());
+}
+
+// DEBUG_ASSERT is slightly slower due to the function call, but gives more
+// detailed info.
+#define DEBUG_ASSERT(condition, EX, msg) \
+  if (!(condition)) AssertFail<EX>(__FILE__, __LINE__, #condition, msg);
+// ASSERT has almost no overhead, so better to use for frequent calls like
+// vector bounds checking.
 #define ASSERT(condition, EX) \
   if (!(condition)) throw(EX);
 #else
+#define DEBUG_ASSERT(condition, EX, msg)
 #define ASSERT(condition, EX)
 #endif
+/** @} */

src/CMakeLists.txt

@@ -105,13 +105,7 @@ target_include_directories(
   PRIVATE ${MANIFOLD_INCLUDE_DIRS}
 )
 
-set(
-  OPTIONS
-  MANIFOLD_EXCEPTIONS
-  MANIFOLD_DEBUG
-  MANIFOLD_CROSS_SECTION
-  MANIFOLD_EXPORT
-)
+set(OPTIONS MANIFOLD_DEBUG MANIFOLD_CROSS_SECTION MANIFOLD_EXPORT)
 foreach(OPT IN LISTS OPTIONS)
   if(${${OPT}})
     target_compile_options(manifold PUBLIC -D${OPT})

src/polygon.cpp

@@ -950,7 +950,7 @@ namespace manifold {
 std::vector<ivec3> TriangulateIdx(const PolygonsIdx &polys, double epsilon) {
   std::vector<ivec3> triangles;
   double updatedEpsilon = epsilon;
-#ifdef MANIFOLD_EXCEPTIONS
+#ifdef MANIFOLD_DEBUG
   try {
 #endif
     if (IsConvex(polys, epsilon)) {  // fast path
@@ -960,7 +960,6 @@ std::vector<ivec3> TriangulateIdx(const PolygonsIdx &polys, double epsilon) {
       triangles = triangulator.Triangulate();
       updatedEpsilon = triangulator.GetPrecision();
     }
-#ifdef MANIFOLD_EXCEPTIONS
 #ifdef MANIFOLD_DEBUG
     if (params.intermediateChecks) {
       CheckTopology(triangles, polys);
@@ -976,9 +975,6 @@ std::vector<ivec3> TriangulateIdx(const PolygonsIdx &polys, double epsilon) {
   } catch (const std::exception &e) {
     PrintFailure(e, polys, triangles, updatedEpsilon);
     throw;
-#else
-  } catch (const std::exception &e) {
-#endif
   }
 #endif
   return triangles;

src/properties.cpp

@@ -192,7 +192,7 @@ bool Manifold::Impl::Is2Manifold() const {
   stable_sort(halfedge.begin(), halfedge.end());
 
   return all_of(
-      countAt(0_uz), countAt(2 * NumEdge() - 1), [halfedge](size_t edge) {
+      countAt(0_uz), countAt(2 * NumEdge() - 1), [&halfedge](size_t edge) {
         const Halfedge h = halfedge[edge];
         if (h.startVert == -1 && h.endVert == -1 && h.pairedHalfedge == -1)
           return true;

src/quickhull.cpp

@@ -60,9 +60,9 @@ size_t MeshBuilder::addFace() {
   if (disabledFaces.size()) {
     size_t index = disabledFaces.back();
     auto& f = faces[index];
-    ASSERT(f.isDisabled(), logicErr("f should be disabled"));
-    ASSERT(!f.pointsOnPositiveSide,
-           logicErr("f should not be on the positive side"));
+    DEBUG_ASSERT(f.isDisabled(), logicErr, "f should be disabled");
+    DEBUG_ASSERT(!f.pointsOnPositiveSide, logicErr,
+                 "f should not be on the positive side");
     f.mostDistantPointDist = 0;
     disabledFaces.pop_back();
     return index;
@@ -204,8 +204,8 @@ HalfEdgeMesh::HalfEdgeMesh(const MeshBuilder& builderObject,
   }
 
   for (auto& halfEdgeIndexFace : halfEdgeIndexFaces) {
-    ASSERT(halfEdgeMapping.count(halfEdgeIndexFace) == 1,
-           logicErr("invalid halfedge mapping"));
+    DEBUG_ASSERT(halfEdgeMapping.count(halfEdgeIndexFace) == 1, logicErr,
+                 "invalid halfedge mapping");
     halfEdgeIndexFace = halfEdgeMapping[halfEdgeIndexFace];
   }
 
@@ -324,7 +324,7 @@ void QuickHull::createConvexHalfedgeMesh() {
 
   // Compute base tetrahedron
   setupInitialTetrahedron();
-  ASSERT(mesh.faces.size() == 4, logicErr("not a tetrahedron"));
+  DEBUG_ASSERT(mesh.faces.size() == 4, logicErr, "not a tetrahedron");
 
   // Init face stack with those faces that have points assigned to them
   faceList.clear();
@@ -354,8 +354,9 @@ void QuickHull::createConvexHalfedgeMesh() {
     auto& tf = mesh.faces[topFaceIndex];
     tf.inFaceStack = 0;
 
-    ASSERT(!tf.pointsOnPositiveSide || tf.pointsOnPositiveSide->size() > 0,
-           logicErr("there should be points on the positive side"));
+    DEBUG_ASSERT(
+        !tf.pointsOnPositiveSide || tf.pointsOnPositiveSide->size() > 0,
+        logicErr, "there should be points on the positive side");
     if (!tf.pointsOnPositiveSide || tf.isDisabled()) {
       continue;
     }
@@ -376,7 +377,7 @@ void QuickHull::createConvexHalfedgeMesh() {
       const auto faceData = possiblyVisibleFaces.back();
       possiblyVisibleFaces.pop_back();
       auto& pvf = mesh.faces[faceData.faceIndex];
-      ASSERT(!pvf.isDisabled(), logicErr("pvf should not be disabled"));
+      DEBUG_ASSERT(!pvf.isDisabled(), logicErr, "pvf should not be disabled");
 
       if (pvf.visibilityCheckedOnIteration == iter) {
         if (pvf.isVisibleFaceOnCurrentIteration) {
@@ -400,8 +401,8 @@ void QuickHull::createConvexHalfedgeMesh() {
           }
           continue;
         }
-        ASSERT(faceData.faceIndex != topFaceIndex,
-               logicErr("face index invalid"));
+        DEBUG_ASSERT(faceData.faceIndex != topFaceIndex, logicErr,
+                     "face index invalid");
       }
 
       // The face is not visible. Therefore, the halfedge we came from is part
@@ -475,7 +476,7 @@ void QuickHull::createConvexHalfedgeMesh() {
       auto t = mesh.disableFace(faceIndex);
       if (t) {
         // Because we should not assign point vectors to faces unless needed...
-        ASSERT(t->size(), logicErr("t should not be empty"));
+        DEBUG_ASSERT(t->size(), logicErr, "t should not be empty");
         disabledFacePointVectors.push_back(std::move(t));
       }
     }
@@ -530,7 +531,8 @@ void QuickHull::createConvexHalfedgeMesh() {
     // Assign points that were on the positive side of the disabled faces to the
     // new faces.
     for (auto& disabledPoints : disabledFacePointVectors) {
-      ASSERT(disabledPoints, logicErr("disabledPoints should not be null"));
+      DEBUG_ASSERT(disabledPoints != nullptr, logicErr,
+                   "disabledPoints should not be null");
       for (const auto& point : *(disabledPoints)) {
         if (point == activePointIndex) {
           continue;
@@ -550,8 +552,8 @@ void QuickHull::createConvexHalfedgeMesh() {
     for (const auto newFaceIndex : newFaceIndices) {
       auto& newFace = mesh.faces[newFaceIndex];
       if (newFace.pointsOnPositiveSide) {
-        ASSERT(newFace.pointsOnPositiveSide->size() > 0,
-               logicErr("there should be points on the positive side"));
+        DEBUG_ASSERT(newFace.pointsOnPositiveSide->size() > 0, logicErr,
+                     "there should be points on the positive side");
         if (!newFace.inFaceStack) {
           faceList.push_back(newFaceIndex);
           newFace.inFaceStack = 1;
@@ -620,10 +622,11 @@ bool QuickHull::reorderHorizonEdges(VecView<size_t>& horizonEdges) {
       return false;
     }
   }
-  ASSERT(mesh.halfedges[horizonEdges[horizonEdges.size() - 1]].endVert ==
-             mesh.halfedges[mesh.halfedges[horizonEdges[0]].pairedHalfedge]
-                 .endVert,
-         logicErr("invalid halfedge"));
+  DEBUG_ASSERT(
+      mesh.halfedges[horizonEdges[horizonEdges.size() - 1]].endVert ==
+          mesh.halfedges[mesh.halfedges[horizonEdges[0]].pairedHalfedge]
+              .endVert,
+      logicErr, "invalid halfedge");
   return true;
 }
 
@@ -680,8 +683,8 @@ void QuickHull::setupInitialTetrahedron() {
                       std::min((size_t)2, vertexCount - 1),
                       std::min((size_t)3, vertexCount - 1));
   }
-  ASSERT(selectedPoints.first != selectedPoints.second,
-         logicErr("degenerate selectedPoints"));
+  DEBUG_ASSERT(selectedPoints.first != selectedPoints.second, logicErr,
+               "degenerate selectedPoints");
 
   // Find the most distant point to the line between the two chosen extreme
   // points.
@@ -730,8 +733,8 @@ void QuickHull::setupInitialTetrahedron() {
   }
 
   // These three points form the base triangle for our tetrahedron.
-  ASSERT(selectedPoints.first != maxI && selectedPoints.second != maxI,
-         logicErr("degenerate selectedPoints"));
+  DEBUG_ASSERT(selectedPoints.first != maxI && selectedPoints.second != maxI,
+               logicErr, "degenerate selectedPoints");
   std::array<size_t, 3> baseTriangle{selectedPoints.first,
                                      selectedPoints.second, maxI};
   const vec3 baseTriangleVertices[] = {originalVertexData[baseTriangle[0]],