Logical vs. bitwise operators. Both are wrong?

Question

When I use the following minimal code in an C++ console application in Visual Studio 2019, I get two warnings, which are completely opposite.

int main()
{
    unsigned char op1 = 0x1;
    unsigned char op2 = 0x3;
    unsigned char result1 = op1 | op2;
    unsigned char result2 = op1 || op2;
}

The warning at unsigned char result1 = op1 | op2; is

lnt-logical-bitwise-mismatch Using bitwise '|' when logical '||' was probably intended.

The warning at unsigned char result2 = op1 || op2; is

lnt-logical-bitwise-mismatch Using logical '||' when bitwise '|' was probably intended.

This is a little bit curious.

My intention was to use the bitwise operator. How could I change the line unsigned char result1 = op1 | op2;, so that the Visual Studio 2019 warning goes away?

The warning is not from the compiler; the output is error-free. Maybe it comes from the ReSharper C++ module or from Visual Studio code analysis.

(Of course I could ignore that warning, but in the original code there are a lot of them, because there a lot of unsigned char bitwise operations.)

Defendant

The char type is very unusual for bitwise operations. Why are your variables char instead of int? – Enormous

@Someprogrammerdude it is same good as any other type – Proudhon

Yes of course. What made VS indicate both lines as potentially wrong? – Defendant

It could be because of the type. It could be because the static analyzer thinks none of the expressions make sense. Or it could be a false positive. When and where are you getting these warnings? From the VS IntelliSense? From the compiler? From some third-party analyzer tool? Have you tried to search for the error in the documentation? What does it say? – Enormous

Have you read learn.microsoft.com/en-us/cpp/ide/… According to the link op1 || op2 should only be used with booleans and op1 | op2 should only be used with integers, not chars – Canticle

@Proudhon no, char does triple duty in C++, whereas int is only a number, char is a number, a letter and also an atom of object representation. It's also implementation-defined as to whether it is signed or unsigned – Deluna

I would find out what tool is actually generating the warning and file a bug report. result1 = op1 | op2; is fine. result2 = op1 || op2; is well formed but clearly wrong and the warning here makes sense – Nashbar

@Deluna - they are number too – Proudhon

@Canticle you can use the logical operators with non-boolean types but 1) it rarely makes sense, and 2) since all non-zero values are true, the test is less efficient. With the bool type, the compiler will use the CPUs underlying bit-wise operator as it only cares about one bit and the others should all be zero and don't need to be tested. – Nashbar

@Nashbar Yes, of course, you can do this. But Microsoft doesn't recommend it and its linter gives you a warning. – Canticle

Yes, I did read the lnt-logical-bitwise-mismatch page. The solution there is "use the right operand for the right type". Which is in my opinion bitwise. The warning comes from IntelliSense. As we see on the page. – Defendant

Another solution is to configure the linter: learn.microsoft.com/en-us/cpp/ide/… – Canticle

Ok so it seems to be a false positve in the case of unsigned char and bitwise operators. Solution 1: ignore, Solution 2: switch off in linters configuration (what probably matches right positives too) – Defendant

@Proudhon that's my point. char is a bad type, because you don't know which of those 3 meanings applies to a given instance. Why do you think std::byte was added? – Deluna

@Defendant it isn't a false-positive. It is a true positive of a style rule you disagree with. – Deluna

It is a false positive because it says '||' was probably intended when || almost certainly wasn't intended. – Nashbar

@Nashbar The rule may be stupid and don't make sense, but it's not a false positive. The rule is clear. Don't use || with chars. If you don't like the linter rule, disable it in the options. – Canticle

@Canticle Then it's badly worded. It should say using "|" with non-integer type and omit the '||' was probably intended – Nashbar

@Nashbar "Only use bitwise operators on integer values." learn.microsoft.com/en-us/cpp/ide/… – Canticle

@Canticle I totally agree with "Don't use || with chars". But why shouldn't I use | with them? – Defendant

"Only use bitwise operators on integer values." directly contradicts "|| was probably intended" – Nashbar

I assume the warning won't occur for unsigned operands, since bitwise operations on unsigned operands to produce an unsigned result is a reasonably common use case. If so, one option to stop the warning with bitwise | might be to explicitly convert operands to unsigned, do the bitwise operation, and explicitly convert the result to unsigned char e.g. unsigned char result1 = (unsigned char)((unsigned)op1 | (unsigned)op2);. If unsigned doesn't work, int might - either way, I suggest this only as a possible workaround for an analyser bug, not as a recommended code practice. – Twibill

I added the following lines, where result3 and result4 dont get a warning, result5 and result6 get the same warnings like result 1 and result2. unsigned short result3 = op1 | op2; unsigned short result4 = op1 || op2; unsigned short op5 = 0x1; unsigned short op6 = 0x3; unsigned short result5 = op5 | op6; unsigned short result6 = op5 || op6; – Defendant

Please don't post code in comments. It's not readable. Edit your question and add it there. – Canticle

The first rule does not know that the second rule trips. The second rule does not know that the first rule trips. The helpful suggestion is baked into each rule as a hint. – Faultfinding

Answer 1

lnt-logical-bitwise-mismatch is a Visual Studio linter rule. It tells you that logical operators should only be used with booleans:

bool op1 = true;
bool op2 = false;
bool result2 = op1 || op2;

And bitwise operators should only be used with integers, not chars:

unsigned int op1 = 0x1;
unsigned int op2 = 0x3;
unsigned int result2 = op1 | op2;

See here for more details.

If you don't want this warning, you can configure the linter in Options -> Text Editor -> C/C++ -> Code Style -> Linter.

I'm not saying that these rules make sense. I'm just answering why you get the squiggles and how you can avoid it. The usefulness of these rules is a different question.

Canticle

really ? What about? int *p; float *y; /* ... */; if(p && y) { do_something_();} – Proudhon

@Proudhon From the posted link: "Only use logical operators on Boolean values." Disable the linter warning, if you don't want it. It's only a linter rule and a recommendation. – Canticle

Not my DB, but the answer sounds as if the warning for | makes sense on unsigned char, but it doesn't (or if it does, I would explain why). – Disproportionate

@Disproportionate The sentence starts with "It tells you that..." after "lnt-logical-bitwise-mismatch is a Visual Studio linter rule." It's not my opinion. It's a fact. I'm not here to discuss whether these rules make sense or not. I want to describe when the squiggles appear and how to disable them. – Canticle

@Canticle In yor answer you are right, unsigned int example doesnt get the warning with bitwise operator. But what maybe a little hint: when I do both (bitwise or logical) operation with unsigned char operands and the result is unsigned short, I do not get any warning at all. So maybe they want to tell me, I would get an overflow with bitwise operation to char result? – Defendant

@jabaa, I'll suggest to add this example to your answer. Both msvc and gcc said char | char is int& which might be more straight forward for future readers. Ref from https://mcmap.net/q/352209/-print-template-typename-at-compile-time – Cumulative

@LouisGo I have no idea how this is related to the problem in the question. Can you elaborate? – Canticle

Does it help if you use uint8_t instead of unsigned char? Since it's a linter rule, it might care about which name is used for the same primitive type. The semantic meaning of uint8_t is that it's an integral type, even though AFAIK in practice it's always unsigned char if it exists, and thus has its often-undesirable may_alias behaviour, as well as being a character type for stuff like std::cout << my_u8; – Patnode

Answer 2

Why are you using either bit wise or logical operators on an unsigned char?

Logical operators are done on booleans, where you want either a true or false value.

Bitwise operators are done on integers or bytes.

Both examples are thus wrong and you correctly get a warning about them, what is misleading is that it is the data type that is the real issue in both cases.

If you were to use uint8_t for all 4, you'd see that the warning for the bitwise operator goes away, but the warning for the logical operator doesn't.

To the compiler (and debugger) 'uint8_t' and 'unsigned char' are the same, to the reader (and the linter) they aren't. If you use the types that people understand, the code more clearly conveys it's meaning to the reader.

int main()
{
   uint8_t  op1 = 0x0;
   uint8_t  op2 = 0x3;
   uint8_t  result1 = op1 | op2;
   bool result2 = op1 || op2;
}

Doorn

But unsigned char is an integer and used to represent bytes, so bitwise operations on it should be fine? – Funkhouser

@CodesInChaos: and MS is telling that you shouldn't use unsigned char to represent bytes, a position which I agree with. If you've got a legacy code base and that's on thousands of different lines, then turn it off and go about your day. If it's a brand new project, leave it on and use something that has the right semantic meaning. – Doorn

What else could you use to represent opaque bytes? uint8_t isn't guaranteed to be a character type (though current compilers treat it as such), so you can run into trouble with type based aliasing (it's a good choice if you want 8-bit integers instead of bytes though). – Funkhouser