From acf341862aacc66204da13bca1063c18bb559710 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 12 Feb 2021 13:27:48 +0100 Subject: [PATCH] AK: Remove operators && and || from DistinctNumeric These don't do short-circuit evaluation, and so I ran into some some very subtle side-effects when converting code to DistinctNumeric. In code like this: MyDistinctNumeric n; if (n && check_thing(n)) return; There would be no short-circuit evaluation if the return type of check_thing() was implicitly convertible to MyDistinctNumeric. Ran into this while making Ext2FS::GroupIndex a DistinctNumeric. --- AK/DistinctNumeric.h | 10 ---------- AK/Tests/TestDistinctNumeric.cpp | 9 --------- 2 files changed, 19 deletions(-) diff --git a/AK/DistinctNumeric.h b/AK/DistinctNumeric.h index ae0c43f320e..dc679f88e77 100644 --- a/AK/DistinctNumeric.h +++ b/AK/DistinctNumeric.h @@ -150,16 +150,6 @@ public: static_assert(Bool, "'!a' is only available for DistinctNumeric types with 'Bool'."); return !this->m_value; } - bool operator&&(const Self& other) const - { - static_assert(Bool, "'a&&b' is only available for DistinctNumeric types with 'Bool'."); - return this->m_value && other.m_value; - } - bool operator||(const Self& other) const - { - static_assert(Bool, "'a||b' is only available for DistinctNumeric types with 'Bool'."); - return this->m_value || other.m_value; - } // Intentionally don't define `operator bool() const` here. C++ is a bit // overzealos, and whenever there would be a type error, C++ instead tries // to convert to a common int-ish type first. `bool` is int-ish, so diff --git a/AK/Tests/TestDistinctNumeric.cpp b/AK/Tests/TestDistinctNumeric.cpp index 0381bb55aad..83104754d60 100644 --- a/AK/Tests/TestDistinctNumeric.cpp +++ b/AK/Tests/TestDistinctNumeric.cpp @@ -123,13 +123,6 @@ TEST_CASE(operator_bool) EXPECT_EQ(!a, true); EXPECT_EQ(!b, false); EXPECT_EQ(!c, false); - EXPECT_EQ(a && b, false); - EXPECT_EQ(a && c, false); - EXPECT_EQ(b && c, true); - EXPECT_EQ(a || a, false); - EXPECT_EQ(a || b, true); - EXPECT_EQ(a || c, true); - EXPECT_EQ(b || c, true); } TEST_CASE(operator_flags) @@ -233,8 +226,6 @@ TEST_CASE(composability) EXPECT_EQ(a >= b, false); // Bool EXPECT_EQ(!a, true); - EXPECT_EQ(a && b, false); - EXPECT_EQ(a || b, true); // Flags EXPECT_EQ(a & b, GeneralNumeric(0)); EXPECT_EQ(a | b, GeneralNumeric(1));