FIX: throw on wrong indices in removeColumns() / removeRows()

MRPT · Oct 6, 2023 · ce2df7d · ce2df7d
1 parent 6aa9fd5
commit ce2df7d
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 0 deletions.
diff --git a/doc/source/doxygen-docs/changelog.md b/doc/source/doxygen-docs/changelog.md
@@ -3,6 +3,7 @@
 # Version 2.10.3: UNRELEASED
 - BUG FIXES:
   - Fix python wrapper FTBFS in armhf and other architectures.
+  - Fix matrices removeColumns() and removeRows() won't throw if user specified a non-existing index.
 
 # Version 2.10.2: Released Oct 5th, 2023
 - Build system:

diff --git a/libs/math/src/MatrixBase_impl.h b/libs/math/src/MatrixBase_impl.h
@@ -43,6 +43,10 @@ void MatrixBase<Scalar, Derived>::removeColumns(
 	std::sort(idxs.begin(), idxs.end());
 	auto itEnd = std::unique(idxs.begin(), idxs.end());
 	idxs.resize(itEnd - idxs.begin());
+	for (const auto idx : idxs)
+	{
+		ASSERT_LT_(idx, mbDerived().cols());
+	}
 	unsafeRemoveColumns(idxs);
 }
 
@@ -70,6 +74,10 @@ void MatrixBase<Scalar, Derived>::removeRows(
 	std::sort(idxs.begin(), idxs.end());
 	auto itEnd = std::unique(idxs.begin(), idxs.end());
 	idxs.resize(itEnd - idxs.begin());
+	for (const auto idx : idxs)
+	{
+		ASSERT_LT_(idx, mbDerived().rows());
+	}
 	unsafeRemoveRows(idxs);
 }
 

diff --git a/libs/math/src/matrix_ops_unittest.cpp b/libs/math/src/matrix_ops_unittest.cpp
@@ -114,3 +114,39 @@ TEST(Matrices, extractSubmatrixSymmetrical)
 		EXPECT_TRUE(E_expected == E);
 	}
 }
+
+TEST(Matrices, removeColumns)
+{
+	for (size_t i = 0; i < 6; i++)
+	{
+		auto M = mrpt::math::CMatrixDouble::Identity(6);
+		EXPECT_EQ(M.cols(), 6);
+		M.removeColumns({i});
+		EXPECT_EQ(M.cols(), 5) << "For {i}=" << i;
+	}
+
+	{
+		auto M = mrpt::math::CMatrixDouble();  // empty
+		EXPECT_EQ(M.cols(), 0);
+		EXPECT_ANY_THROW(M.removeColumns({0}));
+		EXPECT_ANY_THROW(M.removeColumns({1}));
+	}
+}
+
+TEST(Matrices, removeRows)
+{
+	for (size_t i = 0; i < 6; i++)
+	{
+		auto M = mrpt::math::CMatrixDouble::Identity(6);
+		EXPECT_EQ(M.rows(), 6);
+		M.removeRows({i});
+		EXPECT_EQ(M.rows(), 5) << "For {i}=" << i;
+	}
+
+	{
+		auto M = mrpt::math::CMatrixDouble();  // empty
+		EXPECT_EQ(M.rows(), 0);
+		EXPECT_ANY_THROW(M.removeRows({0}));
+		EXPECT_ANY_THROW(M.removeRows({1}));
+	}
+}