Many linters and static analysis tools do flag them though as these days it is not considered good practice.
Really? I can't recall a single case of this with 'bool' in any codebase in my recent memory. I only see it with integers. And even if it's somehow routine for your codebases with bool (why?!), surely it's not routine inside a conditional expression, so at least they can prohibit it there?
The setup is that we have a large number of tiny triangles, we cast a ray in a random direction, and we want to detect whether it intersects any of the triangles. The summarized code looks like this:
for (/* all triangles */) {
auto u = calcU(/*...*/);
if (u < 0 || u > 1) {
continue;
}
auto v = calcV(/*...*/);
if (v < 0 || u + v > 1) {
continue;
}
auto dist = calcD(/*...*/);
if (dist < nearest) {
nearest = dist;
}
}
So what's happening?We calculate the (x,y) coordinates of the point where our ray intersects the plane in which the triangle lies.
We convert those (x,y) coordinates to (u,v) coordinates, where the vectors u and v are parallel to two sides of the triangle. (And equal in length.)
In our transformed (u,v) space, determining whether a point lies inside the triangle is very easy. With the origin at the corner of the triangle from which the u and v sides emanate, a point is out of bounds if its u-coordinate lies outside the interval [0, 1], or if its v-coordinate lies outside [0, 1-u]. That's what the ifs after calcU and calcV are checking. When we detect that a point is out of bounds, we move on to the next triangle.
The triangles are small and the ray is random, so the intersection of the ray with the plane will strike at a random point. It is almost always the case that the point will lie outside the triangle. This means that the full conditional (u < 0 || u > 1) will almost always be false.
But the two subconditions u < 0 and u > 1 will each be true 50% of the time. They are individually impossible to predict, which causes branch prediction on the first one, u < 0, to fail about 50% of the time. But they are jointly easy to predict. There is a huge performance gain from switching to (u < 0 | u > 1) -- in this case, 50% of the time we will do the work of calculating whether u > 1 even though we didn't have to. But the reward we get for that extra work is that branch prediction drops from a 50% failure rate (actually 45%) to a very low failure rate.
Including the v-coordinate makes the full check, ((u < 0) | (u > 1) | (v < 0) | (u + v > 1)), even more easy to predict. We've decided to guarantee that we will always do the full amount of work, and most of it is unnecessary. But it's easier to do 2x or more the amount of work and test a condition that fails consistently than to do less work and keep having to clear the instruction pipeline.
for (auto x : y)
ok &= check(x);
That gets you angry messages from static analysis, so you probably have to write (since there is no corresponding assignment operator) for (auto x : y)
ok = ok && check(x);
...except now you don't actually perform checks after the first failure. That may or may not be intentional (or even confusing - depending on logging). What you'd really need to write to preserve the original logic is for (auto x : y)
ok = check(x) && ok;
Now the `&& ok` is much easier to miss (both in writing and reading) and the intent is much less clear.I really do wonder why the standard doesn't just specify the behavior of bool & bool. Is it just because of holdover from C?
Confused about your question. Did you have a typo? bool & bool is valid in C++.
> What I would like to write in unit tests:
> ok &= check(x);
> That gets you angry messages from static analysis
Of course for that to be a downside someone would have to write tests for security critical code, which obviously did not happen.
Compare `( a() || b() || c() ) ? yes() : no()` to `( a() | b() | c() ) ? yes() : no()`.
In the second case, there are two paths through the code:
a()
b()
c()
yes()
a()
b()
c()
no()
But in the first case, there are more paths: a() /* true */
yes()
a() /* false */
b() /* true */
yes()
a() /* false */
b() /* false */
c() /* true */
yes()
a() /* false */
b() /* false */
c() /* false */
no()
That is because there are fewer branches in the second statement (just one) than there are in the first statement (three)."How did we get here?" and "Are we in the right place?" are distinct questions.
The point of branch coverage is to show when you missed input combinations for your logic. Artificially running all of the branches and then throwing away results just gives you a false sense of security about how much of your logic you covered.
I mean, technically, using bitwise instead of short-circuiting logic doesn't have branches at an assembly code level. But this is pedantry because we're actually trying to check the logic is correct, not that our compiler can correctly translate && into the same Boolean result as &.
Interestingly, the example provided by slavik81 is able to tolerate non-shortcircuiting behavior specifically because no side effects are involved. It's more of a case of "we don't care about the result of this work, but we have to do it anyway, because it's better to do a lot of extra work concurrently than to do only the necessary work sequentially." And doing the extra work is OK because it doesn't have any side effects.
If side effects were involved, you'd be stuck needing to short-circuit despite the fact that it's slower.
There are also areas in cybersecurity where you'd like the time taken for some operation to be insensitive to the input; short-circuiting is bad there too.
The compiler knows that both variants resolve to the same result with no side effects, so it should try to generate whatever code is more efficient. Depending on several factors, that may be machine code that effectively exhibits short circuit behavior or not, for either expression (so even a|b might end up having a short circuit on the lowest level).
This would be different if you, e.g., sprinkle some volatile keywords (which effectively introduces side effects), but then you'd probably still want to take more care and split the expressions up, unless you really don't care about the order of your forced memory accesses (i.e. if I recall correctly, there is no guarantee that a|b will access a first, then b). And that's not even going into barriers.
if(error1|error2)abort();
But I like to write as much like assembly as I can in every language. Each line of code should do one and only one thing.
if (foo && foo->bar) {
// whatever
}
An operator that did not short circuit would have undefined behavior because it might dereference a NULL pointer. if(foo) {
if(foo->bar) {
//whatever
}
}
separating out the null check and the actual conditional. More lines of code and more nesting, yes, but, if you're trying to strictly adhere to a one thing/one line principle, you probably don't care.Short-circuit 'or' is a little harder to avoid (if you specifically want the short-circuit behavior), since you'd have to duplicate the code in the body of the if. But that doesn't come up as often IME.
I imagine the compiler spits out pretty similar code either way. It's more just a recognition of my own limitations... I'm a lot less likely to screw up something up, the more explicitly it's written out.
When I was a younger man, I wouldn't have bothered.
Putting the null check in the same line as the usage means that it is much harder to separate them accidentally.
As I wrote in another comment, this could still be the result of a freak accident and not have shown up in testing, though (but since I know nothing about the process or the code there, it could be anything).
I believe the explanation is:
A && B : compiler will evaluate B only if A==true
A & B : compiler will evaluate both A (safe) and B (unsafe), then perform bitwise operationThere's also linters and other quality control analyzers that will warn on a bitwise operator on booleans.