From 68eba600b1c7e143d63be19a3c8e891ba8ad3502 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Wed, 5 Dec 2007 08:56:42 +0000 Subject: [PATCH] big reorganisation of asserts, so that: 0) asserts are only done in the public API, except for a few ones explicitly named eigen_internal_assert. 1) internal asserts are disabled unless EIGEN_INTERNAL_DEBUGGING is defined. This limits the impact of debugging on performance. 2) no 'unused argument' warnings anymore when compiling with -DNDEBUG --- src/Core/Column.h | 4 +- src/Core/CopyHelper.h | 2 +- src/Core/Matrix.h | 22 +++++++--- src/Core/MatrixBase.h | 48 ++++++++++++++++++---- src/Core/MatrixStorage.h | 26 ++++++------ src/Core/Minor.h | 3 +- src/Core/NumTraits.h | 2 + src/Core/Row.h | 2 +- src/Core/Util.h | 88 ++++++++++++++++++++-------------------- test/main.h | 2 + 10 files changed, 124 insertions(+), 75 deletions(-) diff --git a/src/Core/Column.h b/src/Core/Column.h index 54fe4cf9a..feb90fa4f 100644 --- a/src/Core/Column.h +++ b/src/Core/Column.h @@ -40,7 +40,7 @@ template class Column Column(const MatRef& matrix, int col) : m_matrix(matrix), m_col(col) { - EIGEN_CHECK_COL_RANGE(matrix, col); + assert(col >= 0 && col < matrix.cols()); } Column(const Column& other) @@ -56,14 +56,12 @@ template class Column Scalar& _write(int row, int col) { EIGEN_UNUSED(col); - EIGEN_CHECK_ROW_RANGE(*this, row); return m_matrix.write(row, m_col); } Scalar _read(int row, int col) const { EIGEN_UNUSED(col); - EIGEN_CHECK_ROW_RANGE(*this, row); return m_matrix.read(row, m_col); } diff --git a/src/Core/CopyHelper.h b/src/Core/CopyHelper.h index 72229eac4..75d3af2e8 100644 --- a/src/Core/CopyHelper.h +++ b/src/Core/CopyHelper.h @@ -63,7 +63,7 @@ template template void MatrixBase::_copy_helper(const MatrixBase& other) { - if(SizeAtCompileTime != Dynamic && SizeAtCompileTime <= EIGEN_LOOP_UNROLLING_LIMIT) + if(SizeAtCompileTime != Dynamic && SizeAtCompileTime <= 25) CopyHelperUnroller::run(*this, other); else for(int i = 0; i < rows(); i++) diff --git a/src/Core/Matrix.h b/src/Core/Matrix.h index 9e7fd8467..566c6bbd7 100644 --- a/src/Core/Matrix.h +++ b/src/Core/Matrix.h @@ -51,13 +51,11 @@ class Matrix : public MatrixBase<_Scalar, Matrix<_Scalar, _Rows, _Cols> >, const Scalar& _read(int row, int col) const { - EIGEN_CHECK_RANGES(*this, row, col); return array()[row + col * Storage::_rows()]; } Scalar& _write(int row, int col) { - EIGEN_CHECK_RANGES(*this, row, col); return array()[row + col * Storage::_rows()]; } @@ -80,9 +78,23 @@ class Matrix : public MatrixBase<_Scalar, Matrix<_Scalar, _Rows, _Cols> >, EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Matrix, *=) EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Matrix, /=) - explicit Matrix() : Storage() {} - explicit Matrix(int dim) : Storage(dim) {} - explicit Matrix(int rows, int cols) : Storage(rows, cols) {} + explicit Matrix() : Storage() + { + assert(RowsAtCompileTime > 0 && ColsAtCompileTime > 0); + } + explicit Matrix(int dim) : Storage(dim) + { + assert(dim > 0); + assert((RowsAtCompileTime == 1 + && (ColsAtCompileTime == Dynamic || ColsAtCompileTime == dim)) + || (ColsAtCompileTime == 1 + && (RowsAtCompileTime == Dynamic || RowsAtCompileTime == dim))); + } + explicit Matrix(int rows, int cols) : Storage(rows, cols) + { + assert(rows > 0 && (RowsAtCompileTime == Dynamic || RowsAtCompileTime == rows) + && cols > 0 && (ColsAtCompileTime == Dynamic || ColsAtCompileTime == cols)); + } template Matrix(const MatrixBase& other) : Storage(other.rows(), other.cols()) diff --git a/src/Core/MatrixBase.h b/src/Core/MatrixBase.h index db4c2befb..665229766 100644 --- a/src/Core/MatrixBase.h +++ b/src/Core/MatrixBase.h @@ -52,11 +52,19 @@ template class MatrixBase Scalar& write(int row, int col) { + // from this single point, we can check all writes to all matrix/expression + // types. We only want this however for internal testing, as this is very slow. + eigen_internal_assert(row >= 0 && row < rows() + && col >= 0 && col < cols()); return static_cast(this)->_write(row, col); } Scalar read(int row, int col) const { + // from this single point, we can check all reads to all matrix/expression + // types. We only want this however for internal testing, as this is very slow. + eigen_internal_assert(row >= 0 && row < rows() + && col >= 0 && col < cols()); return static_cast(this)->_read(row, col); } @@ -68,8 +76,8 @@ template class MatrixBase return *static_cast(this); } - //special case of the above template operator=. Strangely, g++ 4.1 failed to use - //that template when OtherDerived == Derived + //special case of the above template operator=, in order to prevent the compiler + //from generating a default operator= (issue hit with g++ 4.1) Derived& operator=(const MatrixBase& other) { assert(rows() == other.rows() && cols() == other.cols()); @@ -149,23 +157,47 @@ template class MatrixBase Derived& operator/=(const std::complex& other); Scalar operator()(int row, int col) const - { return read(row, col); } + { + assert(row >= 0 && row < rows() + && col >= 0 && col < cols()); + return read(row, col); + } Scalar& operator()(int row, int col) - { return write(row, col); } + { + assert(row >= 0 && row < rows() + && col >= 0 && col < cols()); + return write(row, col); + } Scalar operator[](int index) const { assert(IsVector); - if(RowsAtCompileTime == 1) return read(0, index); - else return read(index, 0); + if(RowsAtCompileTime == 1) + { + assert(index >= 0 && index < cols()); + return read(0, index); + } + else + { + assert(index >= 0 && index < rows()); + return read(index, 0); + } } Scalar& operator[](int index) { assert(IsVector); - if(RowsAtCompileTime == 1) return write(0, index); - else return write(index, 0); + if(RowsAtCompileTime == 1) + { + assert(index >= 0 && index < cols()); + return write(0, index); + } + else + { + assert(index >= 0 && index < rows()); + return write(index, 0); + } } Eval eval() const EIGEN_ALWAYS_INLINE; diff --git a/src/Core/MatrixStorage.h b/src/Core/MatrixStorage.h index ec9f10b7f..3fa50eebe 100644 --- a/src/Core/MatrixStorage.h +++ b/src/Core/MatrixStorage.h @@ -35,7 +35,11 @@ class MatrixStorage Scalar m_array[RowsAtCompileTime * ColsAtCompileTime]; void resize(int rows, int cols) - { assert(rows == RowsAtCompileTime && cols == ColsAtCompileTime); } + { + EIGEN_ONLY_USED_FOR_DEBUG(rows); + EIGEN_ONLY_USED_FOR_DEBUG(cols); + assert(rows == RowsAtCompileTime && cols == ColsAtCompileTime); + } int _rows() const { return RowsAtCompileTime; } @@ -46,16 +50,12 @@ class MatrixStorage public: MatrixStorage() {} - MatrixStorage(int dim) - { - assert((RowsAtCompileTime == 1 && ColsAtCompileTime == dim) - || (ColsAtCompileTime == 1 && RowsAtCompileTime == dim)); - } + MatrixStorage(int dim) { EIGEN_UNUSED(dim); } MatrixStorage(int rows, int cols) { - assert(RowsAtCompileTime > 0 && ColsAtCompileTime > 0 - && rows == RowsAtCompileTime && cols == ColsAtCompileTime); + EIGEN_UNUSED(rows); + EIGEN_UNUSED(cols); } ~MatrixStorage() {}; @@ -70,6 +70,7 @@ class MatrixStorage void resize(int rows, int cols) { + EIGEN_ONLY_USED_FOR_DEBUG(cols); assert(rows > 0 && cols == ColsAtCompileTime); if(rows > m_rows) { @@ -88,13 +89,12 @@ class MatrixStorage public: MatrixStorage(int dim) : m_rows(dim) { - assert(m_rows > 0 && ColsAtCompileTime == 1); m_array = new Scalar[m_rows * ColsAtCompileTime]; } MatrixStorage(int rows, int cols) : m_rows(rows) { - assert(m_rows > 0 && cols == ColsAtCompileTime && ColsAtCompileTime > 0); + EIGEN_UNUSED(cols); m_array = new Scalar[m_rows * ColsAtCompileTime]; } @@ -114,6 +114,7 @@ class MatrixStorage void resize(int rows, int cols) { + EIGEN_ONLY_USED_FOR_DEBUG(rows); assert(rows == RowsAtCompileTime && cols > 0); if(cols > m_cols) { @@ -132,13 +133,12 @@ class MatrixStorage public: MatrixStorage(int dim) : m_cols(dim) { - assert(m_cols > 0 && RowsAtCompileTime == 1); m_array = new Scalar[m_cols * RowsAtCompileTime]; } MatrixStorage(int rows, int cols) : m_cols(cols) { - assert(rows == RowsAtCompileTime && RowsAtCompileTime > 0 && cols > 0); + EIGEN_UNUSED(rows); m_array = new Scalar[m_cols * RowsAtCompileTime]; } @@ -175,9 +175,9 @@ class MatrixStorage { return m_cols; } public: + MatrixStorage(int rows, int cols) : m_rows(rows), m_cols(cols) { - assert(m_rows > 0 && m_cols > 0); m_array = new Scalar[m_rows * m_cols]; } diff --git a/src/Core/Minor.h b/src/Core/Minor.h index 9c4116adb..c2b00cbf5 100644 --- a/src/Core/Minor.h +++ b/src/Core/Minor.h @@ -44,7 +44,8 @@ template class Minor int row, int col) : m_matrix(matrix), m_row(row), m_col(col) { - EIGEN_CHECK_RANGES(matrix, row, col); + assert(row >= 0 && row < matrix.rows() + && col >= 0 && col < matrix.cols()); } Minor(const Minor& other) diff --git a/src/Core/NumTraits.h b/src/Core/NumTraits.h index d4563c8e0..75617aed4 100644 --- a/src/Core/NumTraits.h +++ b/src/Core/NumTraits.h @@ -72,6 +72,7 @@ inline int sqrt(const int& x) // (the square root does not always exist within the integers). // Please cast to a floating-point type. assert(false); + return 0; } template<> inline int random() { @@ -158,6 +159,7 @@ inline std::complex sqrt(const std::complex& x) // as this is ambiguous (there are two square roots). // What were you trying to do? assert(false); + return 0; } template<> inline std::complex random() { diff --git a/src/Core/Row.h b/src/Core/Row.h index 89a0dfb25..fc0e2b18e 100644 --- a/src/Core/Row.h +++ b/src/Core/Row.h @@ -40,7 +40,7 @@ template class Row Row(const MatRef& matrix, int row) : m_matrix(matrix), m_row(row) { - EIGEN_CHECK_ROW_RANGE(matrix, row); + assert(row >= 0 && row < matrix.rows()); } Row(const Row& other) diff --git a/src/Core/Util.h b/src/Core/Util.h index aaa94c53d..9820deb8f 100644 --- a/src/Core/Util.h +++ b/src/Core/Util.h @@ -32,13 +32,52 @@ EIGEN_USING_MATRIX_TYPEDEFS \ using Eigen::Matrix; +#ifdef EIGEN_INTERNAL_DEBUGGING +#define eigen_internal_assert(x) assert(x) +#else +#define eigen_internal_assert(x) +#endif + #define EIGEN_UNUSED(x) (void)x -#define EIGEN_CHECK_RANGES(matrix, row, col) \ - assert(row >= 0 && row < (matrix).rows() && col >= 0 && col < (matrix).cols()) -#define EIGEN_CHECK_ROW_RANGE(matrix, row) \ - assert(row >= 0 && row < (matrix).rows()) -#define EIGEN_CHECK_COL_RANGE(matrix, col) \ - assert(col >= 0 && col < (matrix).cols()) + +#ifdef NDEBUG +#define EIGEN_ONLY_USED_FOR_DEBUG(x) EIGEN_UNUSED(x) +#else +#define EIGEN_ONLY_USED_FOR_DEBUG(x) +#endif + +#ifdef __GNUC__ +# define EIGEN_ALWAYS_INLINE __attribute__((always_inline)) +# define EIGEN_RESTRICT /*__restrict__*/ +#else +# define EIGEN_ALWAYS_INLINE +# define EIGEN_RESTRICT +#endif + +#define EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, Op) \ +template \ +Derived& operator Op(const MatrixBase& other) \ +{ \ + return MatrixBase::operator Op(other); \ +} \ +Derived& operator Op(const Derived& other) \ +{ \ + return MatrixBase::operator Op(other); \ +} + +#define EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, Op) \ +template \ +Derived& operator Op(const Other& scalar) \ +{ \ + return MatrixBase::operator Op(scalar); \ +} + +#define EIGEN_INHERIT_ASSIGNMENT_OPERATORS(Derived) \ +EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, =) \ +EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, +=) \ +EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, -=) \ +EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, *=) \ +EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, /=) //forward declarations template class Matrix; @@ -75,41 +114,4 @@ struct ForwardDecl > const int Dynamic = -1; -#define EIGEN_LOOP_UNROLLING_LIMIT 25 - -#define EIGEN_UNUSED(x) (void)x - -#ifdef __GNUC__ -# define EIGEN_ALWAYS_INLINE __attribute__((always_inline)) -# define EIGEN_RESTRICT /*__restrict__*/ -#else -# define EIGEN_ALWAYS_INLINE -# define EIGEN_RESTRICT -#endif - -#define EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, Op) \ -template \ -Derived& operator Op(const MatrixBase& other) \ -{ \ - return MatrixBase::operator Op(other); \ -} \ -Derived& operator Op(const Derived& other) \ -{ \ - return MatrixBase::operator Op(other); \ -} - -#define EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, Op) \ -template \ -Derived& operator Op(const Other& scalar) \ -{ \ - return MatrixBase::operator Op(scalar); \ -} - -#define EIGEN_INHERIT_ASSIGNMENT_OPERATORS(Derived) \ -EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, =) \ -EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, +=) \ -EIGEN_INHERIT_ASSIGNMENT_OPERATOR(Derived, -=) \ -EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, *=) \ -EIGEN_INHERIT_SCALAR_ASSIGNMENT_OPERATOR(Derived, /=) - #endif // EIGEN_UTIL_H diff --git a/test/main.h b/test/main.h index 067b966f9..bf0b263c5 100644 --- a/test/main.h +++ b/test/main.h @@ -27,6 +27,8 @@ #define EIGEN_TEST_MAIN_H #include + +#define EIGEN_INTERNAL_DEBUGGING #include "../src/Core.h" #include