5 ms·

Yeah it has nothing to do with type coercion. I'm baffled a linter doesn't catch this though. & for bools should just be prohibited altogether. If people really

Yeah it has nothing to do with type coercion. I'm baffled a linter doesn't catch this though. & for bools should just be prohibited altogether. If people really want & for bool, they can just cast to an integer type and use & with that (which generates its own warnings appropriately).

gpderetta 5y ago
the problem is that bitwise & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions, so it is hard for the compiler to flag this as an error. Many linters and static analysis tools do flag them though as these days it is not considered good practice.
- dataflow 5y ago
  > & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions 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?
  - kevin_thibedeau 5y ago
    Whenever I see it I assume it's a typo. If it was truly intentional I would expect a comment pointing out why it's needed.
- anyfoo 5y ago
  Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not the most elegant idea.
  - slavik81 5y ago
    Matt Godbolt gave an example of a significant performance hit caused by unnecessary short-circuiting in his CppCon 2019 talk[1]. EDIT: Deleted tangent. [1]: https://youtu.be/HG6c4Kwbv4I?t=45m https://youtu.be/HG6c4Kwbv4I?t=45m
    - thaumasiotes 5y ago
      This example is interesting enough that it deserves to be written up for convenience. The reason there is a performance hit is that the example involves a lot of mispredicted branches. 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.

gpderetta 5y ago

the problem is that bitwise & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions, so it is hard for the compiler to flag this as an error. Many linters and static analysis tools do flag them though as these days it is not considered good practice.

dataflow 5y ago
> & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions 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?
- kevin_thibedeau 5y ago
  Whenever I see it I assume it's a typo. If it was truly intentional I would expect a comment pointing out why it's needed.
anyfoo 5y ago
Why would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not the most elegant idea.
- slavik81 5y ago
  Matt Godbolt gave an example of a significant performance hit caused by unnecessary short-circuiting in his CppCon 2019 talk[1]. EDIT: Deleted tangent. [1]: https://youtu.be/HG6c4Kwbv4I?t=45m https://youtu.be/HG6c4Kwbv4I?t=45m
  - thaumasiotes 5y ago
    This example is interesting enough that it deserves to be written up for convenience. The reason there is a performance hit is that the example involves a lot of mispredicted branches. 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.