From c0b8e584047d5fd6e83a95f0af16c081ba03be9f Mon Sep 17 00:00:00 2001
From: Jonathan G Rennison <j.g.rennison@gmail.com>
Date: Sat, 27 Jan 2024 17:17:27 +0000
Subject: [PATCH] Codechange: Simplify SetBitIterator

Use FindFirstBit and KillFirstBit, allowing simpler iterator equality
Add simple test
---
 src/core/bitmath_func.hpp  | 10 ++++++----
 src/tests/CMakeLists.txt   |  1 +
 src/tests/bitmath_func.cpp | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 src/tests/bitmath_func.cpp

diff --git a/src/core/bitmath_func.hpp b/src/core/bitmath_func.hpp
index c7f7e0c072..ffda4b2eeb 100644
--- a/src/core/bitmath_func.hpp
+++ b/src/core/bitmath_func.hpp
@@ -294,7 +294,7 @@ struct SetBitIterator {
 
 		bool operator==(const Iterator &other) const
 		{
-			return this->bitset == other.bitset && (this->bitset == 0 || this->bitpos == other.bitpos);
+			return this->bitset == other.bitset;
 		}
 		bool operator!=(const Iterator &other) const { return !(*this == other); }
 		Tbitpos operator*() const { return this->bitpos; }
@@ -305,12 +305,14 @@ struct SetBitIterator {
 		Tbitpos bitpos;
 		void Validate()
 		{
-			while (this->bitset != 0 && (this->bitset & 1) == 0) this->Next();
+			if (this->bitset != 0) {
+				typename std::make_unsigned<Tbitset>::type unsigned_value = this->bitset;
+				this->bitpos = static_cast<Tbitpos>(FindFirstBit(unsigned_value));
+			}
 		}
 		void Next()
 		{
-			this->bitset = static_cast<Tbitset>(this->bitset >> 1);
-			this->bitpos++;
+			this->bitset = KillFirstBit(this->bitset);
 		}
 	};
 
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index b8380bed24..53884be7d7 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_test_files(
+    bitmath_func.cpp
     landscape_partial_pixel_z.cpp
     math_func.cpp
     mock_environment.h
diff --git a/src/tests/bitmath_func.cpp b/src/tests/bitmath_func.cpp
new file mode 100644
index 0000000000..fa89f14252
--- /dev/null
+++ b/src/tests/bitmath_func.cpp
@@ -0,0 +1,33 @@
+/*
+ * This file is part of OpenTTD.
+ * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2.
+ * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/** @file bitmath_func.cpp Test functionality from core/bitmath_func. */
+
+#include "../stdafx.h"
+
+#include "../3rdparty/catch2/catch.hpp"
+
+#include "../core/bitmath_func.hpp"
+
+TEST_CASE("SetBitIterator tests")
+{
+	auto test_case = [&](auto input, std::initializer_list<uint> expected) {
+		auto iter = expected.begin();
+		for (auto bit : SetBitIterator(input)) {
+			if (iter == expected.end()) return false;
+			if (bit != *iter) return false;
+			++iter;
+		}
+		return iter == expected.end();
+	};
+	CHECK(test_case(0, {}));
+	CHECK(test_case(1, { 0 }));
+	CHECK(test_case(42, { 1, 3, 5 }));
+	CHECK(test_case(0x8080FFFFU, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 23, 31 }));
+	CHECK(test_case(INT32_MIN, { 31 }));
+	CHECK(test_case(INT64_MIN, { 63 }));
+}