Merge pull request #3548 from opensim-org/fix_issue-3531

Replace uses of bare `assert` with `OPENSIM_ASSERT`

CHANGELOG.md

@@ -15,6 +15,9 @@ been added to these forces to provide access to concrete path types (e.g., `updP
 `getGeometryPath`, `updGeometryPath`, etc., or a suitable alternative.    
 - Fixed a minor memory leak when calling `OpenSim::CoordinateCouplerConstraint::setFunction` (#3541)
 - Increase the number of input dimensions supported by `MultivariatePolynomialFunction` to 6 (#3386)
+- Added `Assertion.h` and associated `OPENSIM_ASSERT*` macros (#3531)
+- Replaced uses of `assert` with `OPENSIM_ASSERT`, so that assertion may be configured via cmake in the future, and
+  so that OpenSim (esp. debug builds) throw instead of terminating the process (#3531)
 - Fixed mis-indexing into an `OpenSim::ObjectProperty` now throws an exception rather than segfaulting (#3347)
 - `PointToPointSpring` now throws an exception on finalizing connections if both sides of the spring
   are connected to the same base frame (#3485)

OpenSim/Actuators/CoordinateActuator.cpp

@@ -25,6 +25,7 @@
 //==============================================================================
 // INCLUDES
 //==============================================================================
+#include <OpenSim/Common/Assertion.h>
 #include <OpenSim/Simulation/Model/Model.h>
 #include <OpenSim/Simulation/Model/CoordinateSet.h>
 #include <OpenSim/Simulation/Model/ForceSet.h>
@@ -206,7 +207,7 @@ void CoordinateActuator::computeForce( const SimTK::State& s,
 double CoordinateActuator::
 getSpeed( const SimTK::State& s) const
 {
-    assert(_coord);
+    OPENSIM_ASSERT_FRMOBJ(_coord != nullptr);
     return _coord->getSpeedValue(s);
 };
 

OpenSim/Auxiliary/auxiliaryTestMuscleFunctions.h

@@ -22,6 +22,7 @@
  * limitations under the License.                                             *
  * -------------------------------------------------------------------------- */
 
+#include "OpenSim/Common/Assertion.h"
 #include "OpenSim/Simulation/Model/ActivationFiberLengthMuscle.h"
 
 /** A debugging utility for investigating muscle equilibrium failures.
@@ -40,7 +41,7 @@ void reportTendonAndFiberForcesAcrossFiberLengths(const T& muscle,
 {
     // should only be using this utility for equilibrium muscles 
     // with a compliant tendon
-    assert(!muscle.get_ignore_tendon_compliance());
+    OPENSIM_ASSERT(!muscle.get_ignore_tendon_compliance());
 
     SimTK::State s = state;
 

OpenSim/Common/AbstractProperty.cpp

@@ -24,6 +24,7 @@
 // INCLUDES
 //============================================================================
 #include "AbstractProperty.h"
+#include "Assertion.h"
 #include "Object.h"
 
 #include <limits>
@@ -205,7 +206,7 @@ void AbstractProperty::writeToXMLParentElement(Xml::Element& parent) const {
     if (!isOneObjectProperty()) {
         // Concrete property will be represented by an Xml element of
         // the form <propName> value(s) </propName>.
-        assert(!getName().empty());
+        OPENSIM_ASSERT(!getName().empty());
         Xml::Element propElement(getName());
         writeToXMLElement(propElement);
         parent.insertNodeAfter(parent.node_end(), propElement);

OpenSim/Common/AbstractProperty.h

@@ -24,7 +24,7 @@
  * -------------------------------------------------------------------------- */
 
 // INCLUDES
-#include <assert.h>
+#include "Assertion.h"
 #include <string>
 #include <typeinfo>
 #include "osimCommonDLL.h"
@@ -79,13 +79,13 @@ class OSIMCOMMON_API AbstractProperty {
     /** Require that the number of values n in the value list of this property
     be in the range aMin <= n <= aMax. */
     void setAllowableListSize(int aMin, int aMax) 
-    {   assert(0 <= aMin && aMin <= aMax); 
+    {   OPENSIM_ASSERT(0 <= aMin && aMin <= aMax);
        _minListSize = aMin; _maxListSize = aMax; }
 
     /** Require that the number of values n in the value list of this property
     be exactly n=aNum values. **/
     void setAllowableListSize(int aNum) 
-    {   assert(aNum >= 1); _minListSize = _maxListSize = aNum; }
+    {   OPENSIM_ASSERT(aNum >= 1); _minListSize = _maxListSize = aNum; }
 
     // Default copy constructor and copy assignment operator.
 

OpenSim/Common/Assertion.cpp

@@ -0,0 +1,41 @@
+/* ------------------------------------------------------------------------- *
+*                           OpenSim:  Assertion.cpp                          *
+* -------------------------------------------------------------------------- *
+* The OpenSim API is a toolkit for musculoskeletal modeling and simulation.  *
+* See http://opensim.stanford.edu and the NOTICE file for more information.  *
+* OpenSim is developed at Stanford University and supported by the US        *
+* National Institutes of Health (U54 GM072970, R24 HD065690) and by DARPA    *
+* through the Warrior Web program.                                           *
+*                                                                            *
+* Copyright (c) 2005-2023 Stanford University and the Authors                *
+* Author(s): Adam Kewley                                                     *
+*                                                                            *
+* Licensed under the Apache License, Version 2.0 (the "License"); you may    *
+* not use this file except in compliance with the License. You may obtain a  *
+* copy of the License at http://www.apache.org/licenses/LICENSE-2.0.         *
+*                                                                            *
+* Unless required by applicable law or agreed to in writing, software        *
+* distributed under the License is distributed on an "AS IS" BASIS,          *
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.   *
+* See the License for the specific language governing permissions and        *
+* limitations under the License.                                             *
+* -------------------------------------------------------------------------- */
+
+#include "Assertion.h"
+
+#include <OpenSim/Common/Exception.h>
+
+void OpenSim::OnAssertionError(
+    char const* failingCode,
+    char const* failingFile,
+    char const* failingFunction,
+    unsigned int failingLine,
+    Object const* maybeSourceObject)
+{
+    if (maybeSourceObject) {
+        throw Exception{failingFile, failingLine, failingFunction, *maybeSourceObject, failingCode};
+    }
+    else {
+        throw Exception{failingFile, failingLine, failingFunction, failingCode};
+    }
+}

OpenSim/Common/Assertion.h

@@ -0,0 +1,74 @@
+#ifndef OPENSIM_ASSERTION_H_
+#define OPENSIM_ASSERTION_H_
+
+/* ------------------------------------------------------------------------- *
+*                           OpenSim:  Assertion.h                            *
+* -------------------------------------------------------------------------- *
+* The OpenSim API is a toolkit for musculoskeletal modeling and simulation.  *
+* See http://opensim.stanford.edu and the NOTICE file for more information.  *
+* OpenSim is developed at Stanford University and supported by the US        *
+* National Institutes of Health (U54 GM072970, R24 HD065690) and by DARPA    *
+* through the Warrior Web program.                                           *
+*                                                                            *
+* Copyright (c) 2005-2023 Stanford University and the Authors                *
+* Author(s): Adam Kewley                                                     *
+*                                                                            *
+* Licensed under the Apache License, Version 2.0 (the "License"); you may    *
+* not use this file except in compliance with the License. You may obtain a  *
+* copy of the License at http://www.apache.org/licenses/LICENSE-2.0.         *
+*                                                                            *
+* Unless required by applicable law or agreed to in writing, software        *
+* distributed under the License is distributed on an "AS IS" BASIS,          *
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.   *
+* See the License for the specific language governing permissions and        *
+* limitations under the License.                                             *
+* -------------------------------------------------------------------------- */
+
+#include "osimCommonDLL.h"
+
+namespace OpenSim {
+
+    class Object;
+
+    // a function that is called whenever an assertion fails
+    //
+    // callers may assume that the implementation halts forward progression of
+    // the caller by either throwing an exception or terminating the process
+    [[noreturn]] OSIMCOMMON_API void OnAssertionError(
+        char const* failingCode,
+        char const* failingFile,
+        char const* failingFunction,
+        unsigned int failingLine,
+        Object const* maybeSourceObject = nullptr
+    );
+}
+
+/**
+ * @name Macros to assert that expressions are true
+ *
+ * The purpose of these macros is to assert that preconditions are met within the
+ * source code of OpenSim. If the precondition is not met, and the macros are enabled,
+ * then the implementation will halt forward progression of the calling thread, either
+ * by throwing an exception or terminating the process.
+ */
+
+// always ensures `expr` converts to `true` or otherwise halts forward progression of the calling thread
+#define OPENSIM_ASSERT_ALWAYS(expr) \
+    static_cast<bool>(expr) ? static_cast<void>(0) : OpenSim::OnAssertionError(#expr, __FILE__, __func__, __LINE__)
+
+// always ensures `expr` converts to `true` or otherwise halts forward progression of the calling thread
+// and passes `this` to the error handler (for better error messages)
+#define OPENSIM_ASSERT_FRMOBJ_ALWAYS(expr) \
+    static_cast<bool>(expr) ? static_cast<void>(0) : OpenSim::OnAssertionError(#expr, __FILE__, __func__, __LINE__, this)
+
+// these macros behave identially to their `_ALWAYS` counterparts, but may be conditionally
+// toggled by compile-time flags
+#if defined(NDEBUG)
+#define OPENSIM_ASSERT(expr)
+#define OPENSIM_ASSERT_FRMOBJ(expr)
+#else
+#define OPENSIM_ASSERT(expr) OPENSIM_ASSERT_ALWAYS(expr)
+#define OPENSIM_ASSERT_FRMOBJ(expr) OPENSIM_ASSERT_FRMOBJ_ALWAYS(expr)
+#endif
+
+#endif // OPENSIM_ASSERTION_H_

OpenSim/Common/DataTable.h

@@ -29,6 +29,7 @@ This file defines the  DataTable_ class, which is used by OpenSim to provide an
 in-memory container for data access and manipulation.                         */
 
 #include "AbstractDataTable.h"
+#include "Assertion.h"
 #include "FileAdapter.h"
 #include "SimTKcommon/internal/BigMatrix.h"
 #include "SimTKcommon/internal/Quaternion.h"
@@ -959,7 +960,7 @@ class DataTable_ : public AbstractDataTable {
         // get copy of labels
         auto labels = getColumnLabels();
 
-        assert(labels.size() == _depData.ncol());
+        OPENSIM_ASSERT(labels.size() == _depData.ncol());
 
         // shift columns unless we're already at the last column
         for (size_t c = index; c < getNumColumns()-1; ++c) {

OpenSim/Common/DebugUtilities.cpp

@@ -22,8 +22,8 @@
 #include "DebugUtilities.h"
 
 #include "Logger.h"
-#include <cassert>
 #include <cstdlib>
+#include <exception>
 #include <fstream>
 #include <iostream>
 #include <sstream>
@@ -38,8 +38,7 @@ void Fatal_Error(const char* msg, const char* function, const char* file,
                                   "line = {})", msg, function, file, line);
     log_critical(str);
     throw std::runtime_error(str);
-    assert(false);
-    exit(1);
+    std::terminate();
 }
 
 /**

OpenSim/Common/Exception.cpp

@@ -29,7 +29,6 @@
 // INCLUDES
 #include <iostream>
 #include <string>
-#include <cassert>
 #include "osimCommonDLL.h"
 #include "Exception.h"
 #include "IO.h"
@@ -58,12 +57,6 @@ exception()
     setMessage(aMsg);
     _file = aFile;
     _line = aLine;
-
-// make it assert false when debugging...
-//#ifndef NDEBUG
-//  print(cout);
-//  assert(false);
-//#endif
 }
 
 namespace {

OpenSim/Common/Object.cpp

@@ -32,6 +32,7 @@
 
 #include "Object.h"
 
+#include "Assertion.h"
 #include "Exception.h"
 #include "IO.h"
 #include "Logger.h"
@@ -1663,7 +1664,7 @@ void Object::setObjectIsUpToDateWithProperties()
 
 void Object::updateFromXMLDocument()
 {
-    assert(_document != nullptr);
+    OPENSIM_ASSERT_FRMOBJ(_document != nullptr);
 
     SimTK::Xml::Element e = _document->getRootDataElement();
     IO::CwdChanger cwd = IO::CwdChanger::changeToParentOf(_document->getFileName());

OpenSim/Common/Object.h

@@ -35,13 +35,13 @@
 
 // INCLUDES
 
+#include "Assertion.h"
 #include "osimCommonDLL.h"
 #include "PropertySet.h"
 #include "PropertyTable.h"
 #include "Property.h"
 
 #include <cstring>
-#include <cassert>
 
 // DISABLES MULTIPLE INSTANTIATION WARNINGS
 
@@ -1345,7 +1345,7 @@ ObjectProperty<T>::isEqualTo(const AbstractProperty& other) const {
     // Property_Deprecated implementation can't copy this flag right.
     if (this->getValueIsDefault() != other.getValueIsDefault())
         return false;
-    assert(this->size() == other.size()); // base class checked
+    OPENSIM_ASSERT(this->size() == other.size()); // base class checked
     const ObjectProperty& otherO = ObjectProperty::getAs(other);
     for (int i=0; i<objects.size(); ++i) {
         const T* const thisp  = objects[i].get();
@@ -1404,11 +1404,11 @@ ObjectProperty<T>::readFromXMLElement
 
         // Create an Object of the element tag's type.
         Object* object = Object::newInstanceOfType(objTypeTag);
-        assert(object); // we just checked above
+        OPENSIM_ASSERT(object); // we just checked above
         object->readObjectFromXMLNodeOrFile(*iter, versionNumber);
 
         T* objectT = dynamic_cast<T*>(object);
-        assert(objectT); // should have worked by construction
+        OPENSIM_ASSERT(objectT); // should have worked by construction
         adoptAndAppendValueVirtual(objectT); // don't copy
     }
 

OpenSim/Common/Property.h

@@ -25,6 +25,7 @@
 
 // INCLUDES
 #include "AbstractProperty.h"
+#include "Assertion.h"
 #include "Exception.h"
 #include "Logger.h"
 
@@ -877,7 +878,7 @@ class SimpleProperty : public Property<T> {
         // Property_Deprecated implementation can't copy this flag right.
         if (this->getValueIsDefault() != other.getValueIsDefault())
             return false;
-        assert(this->size() == other.size()); // base class checked
+        OPENSIM_ASSERT(this->size() == other.size()); // base class checked
         const SimpleProperty& otherS = SimpleProperty::getAs(other);
         for (int i=0; i<values.size(); ++i)
             if (!Property<T>::TypeHelper::isEqual(values[i], otherS.values[i]))

OpenSim/Common/PropertyDblVec.h

@@ -27,6 +27,7 @@
 #pragma warning( disable : 4251 )
 #endif
 
+#include "Assertion.h"
 #include "osimCommonDLL.h"
 #include <string>
 #include "Property_Deprecated.h"
@@ -120,7 +121,7 @@ template<int M> class PropertyDblVec_ : public Property_Deprecated
     using Property_Deprecated::setValue;
     /** set value of this property from an array of doubles of equal or greater length */
     void setValue(const Array<double> &anArray) override {
-        assert(anArray.getSize() >= M);
+        OPENSIM_ASSERT(anArray.getSize() >= M);
         for(int i=0;i<M; i++)
             _dblvec[i] = anArray[i];
     }
@@ -130,7 +131,7 @@ template<int M> class PropertyDblVec_ : public Property_Deprecated
     const SimTK::Vec<M>& getValueDblVec() const {return SimTK::Vec<M>::getAs(&_dblvec[0]); };
     /** set value from double array */ // to be used by the serialization code
     void setValue(int aSize, const double aArray[]) override { 
-        assert(aSize == M);
+        OPENSIM_ASSERT(aSize == M);
         setValue(SimTK::Vec<M>::getAs(aArray));
     };
 #ifndef SWIG

OpenSim/Common/PropertyObj.h

@@ -29,6 +29,7 @@
 
 
 // INCLUDES
+#include "Assertion.h"
 #include "osimCommonDLL.h"
 #include <string>
 #include "Object.h"
@@ -74,11 +75,11 @@ class OSIMCOMMON_API PropertyObj : public Property_Deprecated
     bool isAcceptableObjectTag
         (const std::string& objectTypeTag) const override {return true;}
     const Object& getValueAsObject(int index=-1) const override
-    {   assert(index <= 0); return getValueObj(); }
+    {   OPENSIM_ASSERT(index <= 0); return getValueObj(); }
     Object& updValueAsObject(int index=-1) override
-    {   assert(index <= 0); return getValueObj(); }
+    {   OPENSIM_ASSERT(index <= 0); return getValueObj(); }
     void setValueAsObject(const Object& obj, int index=-1) override
-    {   assert(index <= 0); delete _value; _value=obj.clone(); }
+    {   OPENSIM_ASSERT(index <= 0); delete _value; _value=obj.clone(); }
 
     //--------------------------------------------------------------------------
     // OPERATORS