fix: Bitwise AND of SubscribeOptions

[rbu9fe]

That's a bug fix of the bitwise AND operation of SubscribeOptions returning an enum rather than a bool.

[jktjkt]

Why is that a bug? The intention here was to only allow testing against single bit flags, and in that case the return value's only purpose is, IMHO, to cast it to bool anyway. What's your use case?

[rbu9fe]

I want to get the bitwise AND match between two enums. Use case is to test whether only certain values are set using e.g. if (opts != (opts & (SubscribeOptions::Default | SubscribeOptions::OperMerge))).

The bitwise OR operator returns an enum and I would consider it cleaner design if the bitwise AND would to so as well (considering that bitwise operations typically shouldn't change the data type).

Btw. with that change in the PR testing a single bit could be realized using SubscribeOptions::Default != (opts & SubscribeOptions::OperMerge).

[syyyr]

if (opts != (opts & (SubscribeOptions::Default | SubscribeOptions::OperMerge)))

If you're writing out opts twice anyway, couldn't you just write:

if (opts & SubscribeOptions::Default || opts & SubscribeOptions::OperMerge)

I'm not even sure if I wrote the same thing though.

[rbu9fe]

if (opts != (opts & (SubscribeOptions::Default | SubscribeOptions::OperMerge | SubscribeOptions::Enabled))) would turn into 3 occurrences of opt :-) if (opts & SubscribeOptions::Default || opts & SubscribeOptions::OperMerge || opts & SubscribeOptions::Enabled)

I just think bitwise operators should return the original data type. But never mind, the use case I have can be achieved with the existing implementation.

[jktjkt]

I see the point, OTOH it leads to the need to wrap the "partial bitmasks" as well, and at that point the situation becomes a bit inelegant. I guess that's the price for not-so-real bit flags/enums in both C and C++ :(.

[syyyr]

if (opts != (opts & (SubscribeOptions::Default | SubscribeOptions::OperMerge | SubscribeOptions::Enabled))) would turn into 3 occurrences of opt :-)

I see, that's right. Hm. Well, in my VERY humble opinion, I would say that users would be usually only checking for one or at most two flags at the same time. But on the other hand, is there a downside to operator& returning the enum type? Are the enums implicitly convertible so that you can still use the if statement I wrote? If yes, and there's no other downside, I would say let's merge.

I see the point, OTOH it leads to the need to wrap the "partial bitmasks" as well, and at that point the situation becomes a bit inelegant. I guess that's the price for not-so-real bit flags/enums in both C and C++ :(.

Well, we already do operator| right? And we aren't wrapping partial bitmasks (if I correctly understand, you mean the OR'd enum flags).

[rbu9fe]

No they're not implicitly convertible to bool. Instead of if (opts & SubscribeOptions::Default || opts & SubscribeOptions::OperMerge) you would have to write: if (SubscribeOptions::Default != (opts & SubscribeOptions::Default) || SubscribeOptions::Default != (opts & SubscribeOptions::OperMerge))

That doesn't look very nice... Note that the SubscribeOptions::Default != tests against 0 since its value is 0.

[syyyr]

I don't really develop/use this library anymore, but I'd say, that if (opts & SubscribeOptions::OperMerge) should remain possible.

[jktjkt]

I think that the convenience of making it possible to write if (foo & Bar) far outweighs the inconvenience of having to test for one flag at a time, so I won't be merging this.