[fidl][cpp] Bits are represented with class
Far from a fashion statement, representing bits with class rather than
an enum makes it possible to have a conversion operator to a numerical
type (and hence to bool).
This makes it possible to write `if (my_bits_value) { ...`.
A downside is that doing an exhaustive match against a bits value in a
switch statement is no longer helped by compiler warnings... However,
using a bits value in a switch statement and matching against possible
bits members is likely an incorrect pattern in the first place. (This
revealed a bug fostr's handling of bits values, which will be fixed in a
subsequent CL.)
FIDL-620 #done
FIDL-683 #comment no testing infra, tracked in FIDL-623
FIDL-623 #comment test FIDL-683
Test: added some for new behavior, coverage existed for the rest
Change-Id: I922f3e9943d75d44a57dcfc68ee326378fab0ede
diff --git a/garnet/go/src/fidl/compiler/backend/cpp/templates/bits.tmpl.go b/garnet/go/src/fidl/compiler/backend/cpp/templates/bits.tmpl.go
index 20f6dae..55e4403 100644
--- a/garnet/go/src/fidl/compiler/backend/cpp/templates/bits.tmpl.go
+++ b/garnet/go/src/fidl/compiler/backend/cpp/templates/bits.tmpl.go
@@ -6,13 +6,34 @@
const Bits = `
{{- define "BitsForwardDeclaration" }}
-enum class {{ .Name }} : {{ .Type }} {
+class {{ .Name }} final {
+public:
+ constexpr {{ .Name }}() : value_(0u) {}
+ explicit constexpr {{ .Name }}({{ .Type }} value) : value_(value) {}
+
{{- range .Members }}
- {{ .Name }} = {{ .Value }},
+ const static {{ $.Name }} {{ .Name }};
{{- end }}
+ const static {{ .Name }} mask;
+
+ explicit constexpr inline operator {{ .Type }}() const { return value_; }
+ constexpr inline operator bool() const { return value_; }
+ constexpr inline {{ .Name }} operator~() const;
+ constexpr inline {{ .Name }} operator|(const {{ .Name }}& other) const;
+ constexpr inline {{ .Name }} operator&(const {{ .Name }}& other) const;
+ constexpr inline {{ .Name }} operator^(const {{ .Name }}& other) const;
+ constexpr inline void operator|=(const {{ .Name }}& other);
+ constexpr inline void operator&=(const {{ .Name }}& other);
+ constexpr inline void operator^=(const {{ .Name }}& other);
+
+private:
+ {{ .Type }} value_;
};
-const static {{ .Name }} {{ .MaskName }} = static_cast<{{ .Name }}>({{ .Mask }}u);
+{{- range $member := .Members }}
+constexpr const {{ $.Namespace }}::{{ $.Name }} {{ $.Name }}::{{ $member.Name }} = {{ $.Namespace }}::{{ $.Name }}({{ $member.Value }});
+{{- end }}
+constexpr const {{ .Namespace }}::{{ .Name }} {{ .Name }}::mask = {{ $.Namespace }}::{{ $.Name }}({{ .Mask }}u);
inline zx_status_t Clone({{ .Namespace }}::{{ .Name }} value,
{{ .Namespace }}::{{ .Name }}* result) {
@@ -20,45 +41,38 @@
return ZX_OK;
}
-constexpr inline {{ .Namespace }}::{{ .Name }} operator|({{ .Namespace }}::{{ .Name }} _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- return static_cast<{{ .Namespace }}::{{ .Name }}>(
- static_cast<{{ .Type }}>(_lhs) | static_cast<{{ .Type }}>(_rhs));
+constexpr inline {{ .Namespace }}::{{ .Name }} {{ .Name }}::operator~() const {
+ return {{ $.Namespace }}::{{ $.Name }}(~this->value_ & static_cast<{{ .Type }}>(mask));
}
-constexpr inline {{ .Namespace }}::{{ .Name }}& operator|=({{ .Namespace }}::{{ .Name }}& _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- _lhs = _lhs | _rhs;
- return _lhs;
+constexpr inline {{ .Namespace }}::{{ .Name }} {{ .Name }}::operator|(
+ const {{ .Namespace }}::{{ .Name }}& other) const {
+ return {{ $.Namespace }}::{{ $.Name }}(this->value_ | other.value_);
}
-constexpr inline {{ .Namespace }}::{{ .Name }} operator&({{ .Namespace }}::{{ .Name }} _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- return static_cast<{{ .Namespace }}::{{ .Name }}>(
- static_cast<{{ .Type }}>(_lhs) & static_cast<{{ .Type }}>(_rhs));
+constexpr inline {{ .Namespace }}::{{ .Name }} {{ .Name }}::operator&(
+ const {{ .Namespace }}::{{ .Name }}& other) const {
+ return {{ $.Namespace }}::{{ $.Name }}(this->value_ & other.value_);
}
-constexpr inline {{ .Namespace }}::{{ .Name }}& operator&=({{ .Namespace }}::{{ .Name }}& _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- _lhs = _lhs & _rhs;
- return _lhs;
+constexpr inline {{ .Namespace }}::{{ .Name }} {{ .Name }}::operator^(
+ const {{ .Namespace }}::{{ .Name }}& other) const {
+ return {{ $.Namespace }}::{{ $.Name }}(this->value_ ^ other.value_);
}
-constexpr inline {{ .Namespace }}::{{ .Name }} operator^({{ .Namespace }}::{{ .Name }} _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- return static_cast<{{ .Namespace }}::{{ .Name }}>(
- static_cast<{{ .Type }}>(_lhs) ^ static_cast<{{ .Type }}>(_rhs));
+constexpr inline void {{ .Name }}::operator|=(
+ const {{ .Namespace }}::{{ .Name }}& other) {
+ this->value_ |= other.value_;
}
-constexpr inline {{ .Namespace }}::{{ .Name }}& operator^=({{ .Namespace }}::{{ .Name }}& _lhs,
- {{ .Namespace }}::{{ .Name }} _rhs) {
- _lhs = _lhs ^ _rhs;
- return _lhs;
+constexpr inline void {{ .Name }}::operator&=(
+ const {{ .Namespace }}::{{ .Name }}& other) {
+ this->value_ &= other.value_;
}
-constexpr inline {{ .Namespace }}::{{ .Name }} operator~({{ .Namespace }}::{{ .Name }} _value) {
- return static_cast<{{ .Namespace }}::{{ .Name }}>(
- ~static_cast<{{ .Type }}>(_value) & static_cast<{{ .Type }}>({{ .MaskName }}));
+constexpr inline void {{ .Name }}::operator^=(
+ const {{ .Namespace }}::{{ .Name }}& other) {
+ this->value_ ^= other.value_;
}
{{ end }}
@@ -73,7 +87,7 @@
static void Decode(Decoder* decoder, {{ .Namespace }}::{{ .Name }}* value, size_t offset) {
{{ .Type }} underlying = {};
::fidl::Decode(decoder, &underlying, offset);
- *value = static_cast<{{ .Namespace }}::{{ .Name }}>(underlying);
+ *value = {{ $.Namespace }}::{{ $.Name }}(underlying);
}
};
diff --git a/garnet/go/src/fidl/compiler/backend/goldens/bits.test.test.fidl.json.h.golden b/garnet/go/src/fidl/compiler/backend/goldens/bits.test.test.fidl.json.h.golden
index 0fd058c..2c9fbd9 100644
--- a/garnet/go/src/fidl/compiler/backend/goldens/bits.test.test.fidl.json.h.golden
+++ b/garnet/go/src/fidl/compiler/backend/goldens/bits.test.test.fidl.json.h.golden
@@ -8,12 +8,30 @@
namespace test {
namespace name {
-enum class MyBits : uint32_t {
- MY_FIRST_BIT = 1u,
- MY_OTHER_BIT = 2u,
-};
+class MyBits final {
+public:
+ constexpr MyBits() : value_(0u) {}
+ explicit constexpr MyBits(uint32_t value) : value_(value) {}
+ const static MyBits MY_FIRST_BIT;
+ const static MyBits MY_OTHER_BIT;
+ const static MyBits mask;
-const static MyBits MyBitsMask = static_cast<MyBits>(3u);
+ explicit constexpr inline operator uint32_t() const { return value_; }
+ constexpr inline operator bool() const { return value_; }
+ constexpr inline MyBits operator~() const;
+ constexpr inline MyBits operator|(const MyBits& other) const;
+ constexpr inline MyBits operator&(const MyBits& other) const;
+ constexpr inline MyBits operator^(const MyBits& other) const;
+ constexpr inline void operator|=(const MyBits& other);
+ constexpr inline void operator&=(const MyBits& other);
+ constexpr inline void operator^=(const MyBits& other);
+
+private:
+ uint32_t value_;
+};
+constexpr const ::test::name::MyBits MyBits::MY_FIRST_BIT = ::test::name::MyBits(1u);
+constexpr const ::test::name::MyBits MyBits::MY_OTHER_BIT = ::test::name::MyBits(2u);
+constexpr const ::test::name::MyBits MyBits::mask = ::test::name::MyBits(3u);
inline zx_status_t Clone(::test::name::MyBits value,
::test::name::MyBits* result) {
@@ -21,45 +39,38 @@
return ZX_OK;
}
-constexpr inline ::test::name::MyBits operator|(::test::name::MyBits _lhs,
- ::test::name::MyBits _rhs) {
- return static_cast<::test::name::MyBits>(
- static_cast<uint32_t>(_lhs) | static_cast<uint32_t>(_rhs));
+constexpr inline ::test::name::MyBits MyBits::operator~() const {
+ return ::test::name::MyBits(~this->value_ & static_cast<uint32_t>(mask));
}
-constexpr inline ::test::name::MyBits& operator|=(::test::name::MyBits& _lhs,
- ::test::name::MyBits _rhs) {
- _lhs = _lhs | _rhs;
- return _lhs;
+constexpr inline ::test::name::MyBits MyBits::operator|(
+ const ::test::name::MyBits& other) const {
+ return ::test::name::MyBits(this->value_ | other.value_);
}
-constexpr inline ::test::name::MyBits operator&(::test::name::MyBits _lhs,
- ::test::name::MyBits _rhs) {
- return static_cast<::test::name::MyBits>(
- static_cast<uint32_t>(_lhs) & static_cast<uint32_t>(_rhs));
+constexpr inline ::test::name::MyBits MyBits::operator&(
+ const ::test::name::MyBits& other) const {
+ return ::test::name::MyBits(this->value_ & other.value_);
}
-constexpr inline ::test::name::MyBits& operator&=(::test::name::MyBits& _lhs,
- ::test::name::MyBits _rhs) {
- _lhs = _lhs & _rhs;
- return _lhs;
+constexpr inline ::test::name::MyBits MyBits::operator^(
+ const ::test::name::MyBits& other) const {
+ return ::test::name::MyBits(this->value_ ^ other.value_);
}
-constexpr inline ::test::name::MyBits operator^(::test::name::MyBits _lhs,
- ::test::name::MyBits _rhs) {
- return static_cast<::test::name::MyBits>(
- static_cast<uint32_t>(_lhs) ^ static_cast<uint32_t>(_rhs));
+constexpr inline void MyBits::operator|=(
+ const ::test::name::MyBits& other) {
+ this->value_ |= other.value_;
}
-constexpr inline ::test::name::MyBits& operator^=(::test::name::MyBits& _lhs,
- ::test::name::MyBits _rhs) {
- _lhs = _lhs ^ _rhs;
- return _lhs;
+constexpr inline void MyBits::operator&=(
+ const ::test::name::MyBits& other) {
+ this->value_ &= other.value_;
}
-constexpr inline ::test::name::MyBits operator~(::test::name::MyBits _value) {
- return static_cast<::test::name::MyBits>(
- ~static_cast<uint32_t>(_value) & static_cast<uint32_t>(MyBitsMask));
+constexpr inline void MyBits::operator^=(
+ const ::test::name::MyBits& other) {
+ this->value_ ^= other.value_;
}
} // namespace name
@@ -76,7 +87,7 @@
static void Decode(Decoder* decoder, ::test::name::MyBits* value, size_t offset) {
uint32_t underlying = {};
::fidl::Decode(decoder, &underlying, offset);
- *value = static_cast<::test::name::MyBits>(underlying);
+ *value = ::test::name::MyBits(underlying);
}
};
diff --git a/garnet/public/build/fostr/fostr.fidlmerge b/garnet/public/build/fostr/fostr.fidlmerge
index 623b526..6086c35 100644
--- a/garnet/public/build/fostr/fostr.fidlmerge
+++ b/garnet/public/build/fostr/fostr.fidlmerge
@@ -176,12 +176,17 @@
{{- $type_name := .Name.Parts.Name }}
std::ostream& operator<<(std::ostream& os, const {{ .Name.Parts.Name }}& value) {
using fidl::operator<<;
- switch (value) {
+ bool is_first = true;
{{- range .Members }}
- case {{ $type_name }}::{{ .Name }}:
- return os << "{{ toFriendlyCase .Name }}";
- {{- end }}
+ if (value & {{ $type_name }}::{{ .Name }}) {
+ os << "{{ toFriendlyCase .Name }}";
+ if (is_first) {
+ is_first = false;
+ } else {
+ os << " | ";
+ }
}
+ {{- end }}
return os;
}
diff --git a/sdk/lib/fidl/cpp/bits_unittest.cc b/sdk/lib/fidl/cpp/bits_unittest.cc
index 69cabd9..5294d3d 100644
--- a/sdk/lib/fidl/cpp/bits_unittest.cc
+++ b/sdk/lib/fidl/cpp/bits_unittest.cc
@@ -60,7 +60,19 @@
TEST(Bits, IsConstexpr) {
static constexpr auto this_should_compile = SampleBits::B | SampleBits::D | SampleBits::E;
- EXPECT_EQ(this_should_compile, SampleBitsMask);
+ EXPECT_EQ(this_should_compile, SampleBits::mask);
+}
+
+TEST(Bits, CanConvertToNumberButMustBeExplicit) {
+ uint8_t r8 = static_cast<uint32_t>(SampleBits::B);
+ EXPECT_EQ(r8, 2u);
+ uint16_t r16 = static_cast<uint32_t>(SampleBits::B);
+ EXPECT_EQ(r16, 2u);
+}
+
+TEST(Bits, CanConvertToBool) {
+ bool result = SampleBits::B;
+ EXPECT_TRUE(result);
}
} // namespace
diff --git a/src/connectivity/wlan/drivers/wlanif/convert.cpp b/src/connectivity/wlan/drivers/wlanif/convert.cpp
index 589db09..54fcc00 100644
--- a/src/connectivity/wlan/drivers/wlanif/convert.cpp
+++ b/src/connectivity/wlan/drivers/wlanif/convert.cpp
@@ -991,50 +991,50 @@
}
wlan_mlme::MgmtFrameCaptureFlags ConvertMgmtCaptureFlags(uint32_t ddk_flags) {
- uint32_t ret_flags = 0;
+ wlan_mlme::MgmtFrameCaptureFlags ret_flags;
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_ASSOC_REQ) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::ASSOC_REQ);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::ASSOC_REQ;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_ASSOC_RESP) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::ASSOC_RESP);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::ASSOC_RESP;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_REASSOC_REQ) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::REASSOC_REQ);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::REASSOC_REQ;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_REASSOC_RESP) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::REASSOC_RESP);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::REASSOC_RESP;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_PROBE_REQ) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::PROBE_REQ);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::PROBE_REQ;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_PROBE_RESP) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::PROBE_RESP);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::PROBE_RESP;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_TIMING_AD) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::TIMING_AD);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::TIMING_AD;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_BEACON) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::BEACON);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::BEACON;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_ATIM) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::ATIM);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::ATIM;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_DISASSOC) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::DISASSOC);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::DISASSOC;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_AUTH) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::AUTH);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::AUTH;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_DEAUTH) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::DEAUTH);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::DEAUTH;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_ACTION) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::ACTION);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::ACTION;
}
if ((ddk_flags & WLAN_MGMT_CAPTURE_FLAG_ACTION_NO_ACK) != 0) {
- ret_flags |= static_cast<uint32_t>(wlan_mlme::MgmtFrameCaptureFlags::ACTION_NO_ACK);
+ ret_flags |= wlan_mlme::MgmtFrameCaptureFlags::ACTION_NO_ACK;
}
- return static_cast<wlan_mlme::MgmtFrameCaptureFlags>(ret_flags);
+ return ret_flags;
}
} // namespace wlanif
