Introduce fxcrt::Mask<E> template.
Because C++ just isn't strict enough to catch everything otherwise.
-- Convert one sample usage.
Change-Id: Ife65ff01f54f3e620c788686798359aa741059a9
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/83957
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcrt/BUILD.gn b/core/fxcrt/BUILD.gn
index 8509148..0d38ebd 100644
--- a/core/fxcrt/BUILD.gn
+++ b/core/fxcrt/BUILD.gn
@@ -59,6 +59,7 @@
"fx_types.h",
"fx_unicode.cpp",
"fx_unicode.h",
+ "mask.h",
"maybe_owned.h",
"observed_ptr.cpp",
"observed_ptr.h",
@@ -158,6 +159,7 @@
"fx_random_unittest.cpp",
"fx_string_unittest.cpp",
"fx_system_unittest.cpp",
+ "mask_unittest.cpp",
"maybe_owned_unittest.cpp",
"observed_ptr_unittest.cpp",
"pdfium_span_unittest.cpp",
diff --git a/core/fxcrt/mask.h b/core/fxcrt/mask.h
new file mode 100644
index 0000000..a5d8129
--- /dev/null
+++ b/core/fxcrt/mask.h
@@ -0,0 +1,118 @@
+// Copyright 2021 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CORE_FXCRT_MASK_H_
+#define CORE_FXCRT_MASK_H_
+
+#include <type_traits>
+
+namespace fxcrt {
+
+// Provides extremely strict type-checking on masks of enum class bitflags,
+// for code where flags may not be passed consistently.
+template <typename E>
+class Mask {
+ public:
+ using UnderlyingType = typename std::underlying_type<E>::type;
+
+ // Escape hatch for when value comes aross an API, say.
+ static Mask FromUnderlyingUnchecked(UnderlyingType val) { return Mask(val); }
+
+ constexpr Mask() = default;
+ constexpr Mask(const Mask& that) = default;
+
+ // NOLINTNEXTLINE(runtime/explicit)
+ constexpr Mask(E val) : val_(static_cast<UnderlyingType>(val)) {}
+
+ // Unfortunately, std::initializer_list<> can't be used in constexpr
+ // methods per C++ standards, and we need constexpr for a zero-cost
+ // abstraction. Hence, expand out constructors of various arity.
+ constexpr Mask(E v1, E v2)
+ : val_(static_cast<UnderlyingType>(v1) |
+ static_cast<UnderlyingType>(v2)) {}
+
+ constexpr Mask(E v1, E v2, E v3)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3)) {}
+
+ constexpr Mask(E v1, E v2, E v3, E v4)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3) |
+ static_cast<UnderlyingType>(v4)) {}
+
+ constexpr Mask(E v1, E v2, E v3, E v4, E v5)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3) | static_cast<UnderlyingType>(v4) |
+ static_cast<UnderlyingType>(v5)) {}
+
+ constexpr Mask(E v1, E v2, E v3, E v4, E v5, E v6)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3) | static_cast<UnderlyingType>(v4) |
+ static_cast<UnderlyingType>(v5) |
+ static_cast<UnderlyingType>(v6)) {}
+
+ constexpr Mask(E v1, E v2, E v3, E v4, E v5, E v6, E v7)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3) | static_cast<UnderlyingType>(v4) |
+ static_cast<UnderlyingType>(v5) | static_cast<UnderlyingType>(v6) |
+ static_cast<UnderlyingType>(v7)) {}
+
+ constexpr Mask(E v1, E v2, E v3, E v4, E v5, E v6, E v7, E v8)
+ : val_(static_cast<UnderlyingType>(v1) | static_cast<UnderlyingType>(v2) |
+ static_cast<UnderlyingType>(v3) | static_cast<UnderlyingType>(v4) |
+ static_cast<UnderlyingType>(v5) | static_cast<UnderlyingType>(v6) |
+ static_cast<UnderlyingType>(v7) |
+ static_cast<UnderlyingType>(v8)) {}
+
+ explicit operator bool() const { return !!val_; }
+ Mask operator~() const { return Mask(~val_); }
+ constexpr Mask operator|(const Mask& that) const {
+ return Mask(val_ | that.val_);
+ }
+ constexpr Mask operator&(const Mask& that) const {
+ return Mask(val_ & that.val_);
+ }
+ constexpr Mask operator^(const Mask& that) const {
+ return Mask(val_ ^ that.val_);
+ }
+ Mask& operator=(const Mask& that) {
+ val_ = that.val_;
+ return *this;
+ }
+ Mask& operator|=(const Mask& that) {
+ val_ |= that.val_;
+ return *this;
+ }
+ Mask& operator&=(const Mask& that) {
+ val_ &= that.val_;
+ return *this;
+ }
+ Mask& operator^=(const Mask& that) {
+ val_ ^= that.val_;
+ return *this;
+ }
+ bool operator==(const Mask& that) const { return val_ == that.val_; }
+ bool operator!=(const Mask& that) const { return val_ != that.val_; }
+
+ bool TestAll(const Mask& that) const {
+ return (val_ & that.val_) == that.val_;
+ }
+
+ // Because ~ can't be applied to enum class without casting.
+ void Clear(const Mask& that) { val_ &= ~that.val_; }
+
+ // Escape hatch, usage should be minimized.
+ UnderlyingType UncheckedValue() const { return val_; }
+
+ private:
+ explicit constexpr Mask(UnderlyingType val) : val_(val) {}
+
+ UnderlyingType val_ = 0;
+};
+
+} // namespace fxcrt
+
+using fxcrt::Mask;
+
+#endif // CORE_FXCRT_MASK_H_
diff --git a/core/fxcrt/mask_unittest.cpp b/core/fxcrt/mask_unittest.cpp
new file mode 100644
index 0000000..c0d442f
--- /dev/null
+++ b/core/fxcrt/mask_unittest.cpp
@@ -0,0 +1,170 @@
+// Copyright 2021 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxcrt/mask.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace fxcrt {
+namespace {
+
+enum class Privilege : uint8_t {
+ kPriv1 = 1 << 0,
+ kPriv2 = 1 << 1,
+ kPriv4 = 1 << 2,
+ kPriv8 = 1 << 3,
+ kPriv16 = 1 << 4,
+ kPriv32 = 1 << 5,
+ kPriv64 = 1 << 6,
+ kPriv128 = 1 << 7,
+};
+
+constexpr Mask<Privilege> kAllMask = {
+ Privilege::kPriv1, Privilege::kPriv2, Privilege::kPriv4,
+ Privilege::kPriv8, Privilege::kPriv16, Privilege::kPriv32,
+ Privilege::kPriv64, Privilege::kPriv128,
+};
+
+} // namespace
+
+static_assert(sizeof(Mask<Privilege>) == sizeof(Privilege),
+ "Mask size must be the same as enum");
+
+TEST(Mask, Empty) {
+ constexpr Mask<Privilege> privs;
+ EXPECT_EQ(0u, privs.UncheckedValue());
+ EXPECT_FALSE(privs & Privilege::kPriv1);
+ EXPECT_FALSE(privs & Privilege::kPriv4);
+ EXPECT_FALSE(privs & Privilege::kPriv8);
+ EXPECT_FALSE(privs & kAllMask);
+}
+
+TEST(Mask, FromOne) {
+ Mask<Privilege> privs = Privilege::kPriv1;
+ EXPECT_EQ(1u, privs.UncheckedValue());
+ EXPECT_TRUE(privs & Privilege::kPriv1);
+ EXPECT_FALSE(privs & Privilege::kPriv4);
+ EXPECT_FALSE(privs & Privilege::kPriv8);
+ EXPECT_TRUE(privs & kAllMask);
+}
+
+TEST(Mask, FromTwo) {
+ // Not adjacent bits, just because.
+ Mask<Privilege> privs = {Privilege::kPriv1, Privilege::kPriv8};
+ EXPECT_EQ(9u, privs.UncheckedValue());
+ EXPECT_TRUE(privs & Privilege::kPriv1);
+ EXPECT_FALSE(privs & Privilege::kPriv4);
+ EXPECT_TRUE(privs & Privilege::kPriv8);
+ EXPECT_TRUE(privs & kAllMask);
+}
+
+TEST(Mask, FromThree) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1,
+ Privilege::kPriv2,
+ Privilege::kPriv4,
+ };
+ EXPECT_EQ(7u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromFour) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1,
+ Privilege::kPriv2,
+ Privilege::kPriv4,
+ Privilege::kPriv8,
+ };
+ EXPECT_EQ(15u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromFive) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1, Privilege::kPriv2, Privilege::kPriv4,
+ Privilege::kPriv8, Privilege::kPriv16,
+ };
+ EXPECT_EQ(31u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromSix) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1, Privilege::kPriv2, Privilege::kPriv4,
+ Privilege::kPriv8, Privilege::kPriv16, Privilege::kPriv32,
+ };
+ EXPECT_EQ(63u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromSeven) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1, Privilege::kPriv2, Privilege::kPriv4,
+ Privilege::kPriv8, Privilege::kPriv16, Privilege::kPriv32,
+ Privilege::kPriv64,
+ };
+ EXPECT_EQ(127u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromEight) {
+ Mask<Privilege> privs = {
+ Privilege::kPriv1, Privilege::kPriv2, Privilege::kPriv4,
+ Privilege::kPriv8, Privilege::kPriv16, Privilege::kPriv32,
+ Privilege::kPriv64, Privilege::kPriv128,
+ };
+ EXPECT_EQ(255u, privs.UncheckedValue());
+}
+
+TEST(Mask, FromUnderlying) {
+ auto privs = Mask<Privilege>::FromUnderlyingUnchecked(5);
+ EXPECT_EQ(5u, privs.UncheckedValue());
+ EXPECT_TRUE(privs & Privilege::kPriv1);
+ EXPECT_TRUE(privs & Privilege::kPriv4);
+ EXPECT_FALSE(privs & Privilege::kPriv8);
+}
+
+TEST(Mask, AssignAndEQ) {
+ Mask<Privilege> source = {Privilege::kPriv1, Privilege::kPriv8};
+ Mask<Privilege> other = Privilege::kPriv1;
+ Mask<Privilege> dest;
+ dest = source;
+ EXPECT_EQ(9u, dest.UncheckedValue());
+ EXPECT_EQ(source, dest);
+ EXPECT_NE(other, dest);
+}
+
+TEST(Mask, OrAndAnd) {
+ Mask<Privilege> source = {Privilege::kPriv1, Privilege::kPriv8};
+ Mask<Privilege> or_result =
+ source | Mask<Privilege>{Privilege::kPriv1, Privilege::kPriv4};
+ Mask<Privilege> and_result =
+ source & Mask<Privilege>{Privilege::kPriv1, Privilege::kPriv4};
+ EXPECT_EQ(13u, or_result.UncheckedValue());
+ EXPECT_EQ(1u, and_result.UncheckedValue());
+}
+
+TEST(Mask, OrEqualsAndAndEquals) {
+ Mask<Privilege> source_or = {Privilege::kPriv1, Privilege::kPriv8};
+ Mask<Privilege> source_and = {Privilege::kPriv1, Privilege::kPriv8};
+ source_or |= {Privilege::kPriv1, Privilege::kPriv4};
+ source_and &= {Privilege::kPriv1, Privilege::kPriv4};
+ EXPECT_EQ(13u, source_or.UncheckedValue());
+ EXPECT_EQ(1u, source_and.UncheckedValue());
+}
+
+TEST(Mask, Clear) {
+ Mask<Privilege> source = kAllMask;
+ source.Clear({Privilege::kPriv1, Privilege::kPriv4});
+ EXPECT_EQ(250u, source.UncheckedValue());
+}
+
+TEST(Mask, TestAll) {
+ Mask<Privilege> source = {
+ Privilege::kPriv1,
+ Privilege::kPriv8,
+ Privilege::kPriv64,
+ };
+ Mask<Privilege> passes = {Privilege::kPriv1, Privilege::kPriv64};
+ Mask<Privilege> fails = {Privilege::kPriv1, Privilege::kPriv32};
+ EXPECT_TRUE(source.TestAll(passes));
+ EXPECT_FALSE(source.TestAll(fails));
+}
+
+} // namespace fxcrt
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
index 27ef26a..525f139 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
@@ -651,7 +651,7 @@
}
FWL_KeyFlagMask CPDFXFA_WidgetHandler::GetKeyFlags(FWL_EventFlagMask dwFlag) {
- FWL_KeyFlagMask dwFWLFlag = 0;
+ FWL_KeyFlagMask dwFWLFlag;
if (dwFlag & FWL_EVENTFLAG_ControlKey)
dwFWLFlag |= FWL_KEYFLAG_Ctrl;
diff --git a/xfa/fwl/cfwl_edit.cpp b/xfa/fwl/cfwl_edit.cpp
index 1c3fb9e..a43f9f8 100644
--- a/xfa/fwl/cfwl_edit.cpp
+++ b/xfa/fwl/cfwl_edit.cpp
@@ -40,9 +40,9 @@
constexpr int kEditMargin = 3;
#if defined(OS_APPLE)
-constexpr FWL_KeyFlagMask kEditingModifier = FWL_KEYFLAG_Command;
+constexpr FWL_KeyFlag kEditingModifier = FWL_KEYFLAG_Command;
#else
-constexpr FWL_KeyFlagMask kEditingModifier = FWL_KEYFLAG_Ctrl;
+constexpr FWL_KeyFlag kEditingModifier = FWL_KEYFLAG_Ctrl;
#endif
} // namespace
diff --git a/xfa/fwl/cfwl_message.h b/xfa/fwl/cfwl_message.h
index 0e4c65d..3027c45 100644
--- a/xfa/fwl/cfwl_message.h
+++ b/xfa/fwl/cfwl_message.h
@@ -9,6 +9,7 @@
#include <type_traits>
+#include "core/fxcrt/mask.h"
#include "core/fxcrt/unowned_ptr.h"
#include "v8/include/cppgc/macros.h"
@@ -23,7 +24,7 @@
FWL_KEYFLAG_RButton = 1 << 5,
FWL_KEYFLAG_MButton = 1 << 6
};
-using FWL_KeyFlagMask = std::underlying_type<FWL_KeyFlag>::type;
+using FWL_KeyFlagMask = Mask<FWL_KeyFlag>;
class CFWL_Message {
CPPGC_STACK_ALLOCATED(); // Allow Raw/Unowned pointers.
diff --git a/xfa/fwl/cfwl_messagemouse.h b/xfa/fwl/cfwl_messagemouse.h
index cfeb5b4..05e4e49 100644
--- a/xfa/fwl/cfwl_messagemouse.h
+++ b/xfa/fwl/cfwl_messagemouse.h
@@ -32,7 +32,7 @@
~CFWL_MessageMouse() override;
const MouseCommand m_dwCmd;
- FWL_KeyFlagMask m_dwFlags = 0;
+ FWL_KeyFlagMask m_dwFlags;
CFX_PointF m_pos;
};
diff --git a/xfa/fwl/cfwl_notedriver.cpp b/xfa/fwl/cfwl_notedriver.cpp
index d2a2059..f39fb37 100644
--- a/xfa/fwl/cfwl_notedriver.cpp
+++ b/xfa/fwl/cfwl_notedriver.cpp
@@ -234,8 +234,8 @@
CFWL_MessageMouse* pMsg = static_cast<CFWL_MessageMouse*>(pMessage);
if (m_pHover) {
CFWL_MessageMouse msLeave(
- m_pHover.Get(), CFWL_MessageMouse::MouseCommand::kLeave, 0,
- pTarget->TransformTo(m_pHover.Get(), pMsg->m_pos));
+ m_pHover.Get(), CFWL_MessageMouse::MouseCommand::kLeave,
+ FWL_KeyFlagMask(), pTarget->TransformTo(m_pHover.Get(), pMsg->m_pos));
DispatchMessage(&msLeave, nullptr);
}
if (pTarget->GetClassID() == FWL_Type::Form) {
@@ -244,8 +244,8 @@
}
m_pHover = pTarget;
- CFWL_MessageMouse msHover(pTarget, CFWL_MessageMouse::MouseCommand::kHover, 0,
- pMsg->m_pos);
+ CFWL_MessageMouse msHover(pTarget, CFWL_MessageMouse::MouseCommand::kHover,
+ FWL_KeyFlagMask(), pMsg->m_pos);
DispatchMessage(&msHover, nullptr);
}
diff --git a/xfa/fxfa/cxfa_fffield.cpp b/xfa/fxfa/cxfa_fffield.cpp
index e8cc8c8..bd540e8 100644
--- a/xfa/fxfa/cxfa_fffield.cpp
+++ b/xfa/fxfa/cxfa_fffield.cpp
@@ -372,8 +372,8 @@
return false;
CFWL_MessageMouse msg(GetNormalWidget(),
- CFWL_MessageMouse::MouseCommand::kEnter, 0,
- CFX_PointF());
+ CFWL_MessageMouse::MouseCommand::kEnter,
+ FWL_KeyFlagMask(), CFX_PointF());
SendMessageToFWLWidget(&msg);
return true;
}
@@ -383,8 +383,8 @@
return false;
CFWL_MessageMouse msg(GetNormalWidget(),
- CFWL_MessageMouse::MouseCommand::kLeave, 0,
- CFX_PointF());
+ CFWL_MessageMouse::MouseCommand::kLeave,
+ FWL_KeyFlagMask(), CFX_PointF());
SendMessageToFWLWidget(&msg);
return true;
}
