Skip to content

Base: Enhance Bitmask with constexpr support and exhaustive tests#9

Open
dragonfruit-blue wants to merge 1 commit intoswe-productivity:mainfrom
dragonfruit-blue:issue-3
Open

Base: Enhance Bitmask with constexpr support and exhaustive tests#9
dragonfruit-blue wants to merge 1 commit intoswe-productivity:mainfrom
dragonfruit-blue:issue-3

Conversation

@dragonfruit-blue
Copy link
Copy Markdown

This PR modernizes the Base::Flags class with constexpr support, fixed operator!, and new operators (^, ^=, ==, !=). It also adds an exhaustive test suite in tests/src/Base/Bitmask.cpp to ensure robustness and prevent regressions.


// ---- Const correctness ------------------------------------------------------

TEST_F(BitmaskTest, constCorrectness)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it we are testing here?

Comment thread src/Base/Bitmask.h
}
constexpr bool operator!=(const Enum &f) const {
return i != f;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary -- we target C++23, which will autogenerate operator!=.

@@ -18,14 +59,433 @@ class BitmaskTest: public ::testing::Test
// void TearDown() override {};
};

TEST_F(BitmaskTest, toUnderlyingType)
// ---- Alternative underlying types -------------------------------------------
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a little under-tested still. With a longer type you're worried about truncation along some code path that didn't consider the use of something with so many bits available. It could be future work, but since this is all LLM-generated we may as well do it now.


// Assert
EXPECT_EQ(typeid(result), typeid(int));
TEST_F(BitmaskTest, testFlagAll)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably also test the reverse (that is, make sure it returns false when NOT all the flags are set).

EXPECT_TRUE(flags.testFlag(TestFlagEnum::Flag1));
}

TEST_F(BitmaskTest, setFlagOnDefaultParamIsTrue)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from the test right above it (besides just testing a different flag)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants