24

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.)

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
woelfchen42
  • 419
  • 2
  • 8
  • 1
    The `char` type is very unusual for bitwise operations. Why are your variables `char` instead of `int`? – Some programmer dude Aug 25 '23 at 10:10
  • @Someprogrammerdude it is same good as any other type – 0___________ Aug 25 '23 at 10:11
  • Yes of course. What made VS indicate both lines as potentially wrong? – woelfchen42 Aug 25 '23 at 10:12
  • 6
    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? – Some programmer dude Aug 25 '23 at 10:15
  • 3
    Have you read https://learn.microsoft.com/en-us/cpp/ide/lnt-logical-bitwise-mismatch?view=msvc-170 According to the link `op1 || op2` should only be used with booleans and `op1 | op2` should only be used with integers, not chars – jabaa Aug 25 '23 at 10:21
  • @0___________ 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 – Caleth Aug 25 '23 at 10:22
  • 1
    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 – Rodney Aug 25 '23 at 10:22
  • @Caleth - they are number too – 0___________ Aug 25 '23 at 10:24
  • @jabaa 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. – Rodney Aug 25 '23 at 10:25
  • 2
    @Rodney Yes, of course, you can do this. But Microsoft doesn't recommend it and its linter gives you a warning. – jabaa Aug 25 '23 at 10:26
  • 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. – woelfchen42 Aug 25 '23 at 10:26
  • 1
    Another solution is to configure the linter: https://learn.microsoft.com/en-us/cpp/ide/cpp-linter-overview?view=msvc-170#configure-the-linter – jabaa Aug 25 '23 at 10:29
  • 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) – woelfchen42 Aug 25 '23 at 10:32
  • @0___________ 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? – Caleth Aug 25 '23 at 10:36
  • 6
    @woelfchen42 it isn't a false-positive. It is a true positive of a style rule you disagree with. – Caleth Aug 25 '23 at 10:39
  • It is a false positive because it says `'||' was probably intended` when || almost certainly wasn't intended. – Rodney Aug 25 '23 at 10:46
  • 1
    @Rodney 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. – jabaa Aug 25 '23 at 10:48
  • @jabaa Then it's badly worded. It should say `using "|" with non-integer type` and omit the `'||' was probably intended` – Rodney Aug 25 '23 at 10:52
  • @Rodney _"Only use bitwise operators on integer values."_ https://learn.microsoft.com/en-us/cpp/ide/lnt-logical-bitwise-mismatch?view=msvc-170 – jabaa Aug 25 '23 at 10:53
  • @jabaa I totally agree with "Don't use || with chars". But why shouldn't I use | with them? – woelfchen42 Aug 25 '23 at 10:53
  • "Only use bitwise operators on integer values." directly contradicts "|| was probably intended" – Rodney Aug 25 '23 at 10:54
  • 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. – Peter Aug 25 '23 at 11:09
  • 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;``` – woelfchen42 Aug 25 '23 at 11:29
  • 3
    Please don't post code in comments. It's not readable. Edit your question and add it there. – jabaa Aug 25 '23 at 11:36
  • 1
    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. – Eljay Aug 25 '23 at 11:42

2 Answers2

34

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.

user16217248
  • 3,119
  • 19
  • 19
  • 37
jabaa
  • 5,844
  • 3
  • 9
  • 30
  • 1
    really ? What about? `int *p; float *y; /* ... */; if(p && y) { do_something_();}` – 0___________ Aug 25 '23 at 10:26
  • 2
    @0___________ 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. – jabaa Aug 25 '23 at 10:27
  • 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). – HolyBlackCat Aug 25 '23 at 10:42
  • 3
    @HolyBlackCat 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. – jabaa Aug 25 '23 at 10:43
  • @jabaa 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? – woelfchen42 Aug 25 '23 at 11:46
  • 1
    @jabaa, I'll suggest to add this [example](https://godbolt.org/z/fY93cGdbv) to your answer. Both msvc and gcc said `char | char` is `int&` which might be more straight forward for future readers. Ref from https://stackoverflow.com/a/46339450/4123703 – Louis Go Aug 25 '23 at 14:07
  • @LouisGo I have no idea how this is related to the problem in the question. Can you elaborate? – jabaa Aug 25 '23 at 16:18
  • 13
    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;` – Peter Cordes Aug 25 '23 at 19:10
-5

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;
}
jmoreno
  • 12,752
  • 4
  • 60
  • 91
  • 4
    But `unsigned char` is an integer and used to represent bytes, so bitwise operations on it should be fine? – CodesInChaos Aug 27 '23 at 09:55
  • @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. – jmoreno Sep 01 '23 at 12:57
  • 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). – CodesInChaos Sep 01 '23 at 15:03