Hacker News Reader

Comment by cesarb | Hacker News Reader

Since nobody seems to have done the traditional "blame the programming language" yet, let me get it out of the way: this particular bug wouldn't have happened in a language which doesn't have implicit conversion between boolean and number. The compiler would reject the code both because the numeric result of ".count(...)" is being treated as a boolean by the "&&", and because the boolean result of the "&&" is being treated as a number by the "== 0".

One example of a language without that implicit conversion is Java. It's fairly annoying to have to put a "!= 0" whenever you want to convert a number to a boolean, but it does avoid a lot of programming mistakes within if conditions.

View on HN

That's the way to go.

But it is sad that in $current_year, we're still fighting boring, solved problems.

Meanwhile in Python land, this is not a common class of bugs even though this implicit boolification happens.

My only explanation for that may be because Python has more severe and commonly occuring issues?

I've been bitten by it in Python. It isn't quite up there with PHP style weak typing combining int and string with + but feels similar, even if you can make some modulo 2 arithmetic argument that it should work like other mixed precision stuff.

I wrote a (len(foo == 0)) instead of (len(foo) == 0) in Python yesterday. I don't understand why you would say this.

… which is exactly what you don’t need to do in Python, because that is just `if foo`, thus sidestepping the issue

Lol python is the king of these types of errors. People chose the unsafe because it’s easy when we know types are one of the best tools engineers can have.

Static typing is orthogonal to this issue though. This is more about strong typing. Even a statically typed language could coerce values, e.g. the empty string, to a Boolean false at runtime. I agree that that’s undesirable though. Static and strong typing is the driest, least flexible, but also easiest to get correct way.

If only there was a language that defines explicit conversions between booleans and numbers.

Don't they all have == and != though?

Sure, but in say Rust we can write for example ((v.is_empty() as i16) - 10) and Rust is OK with that, and you get either -9 if it was empty or -10 if it was not empty whereas if you didn't have that explicit cast, that's a bool not an integer, and the bool type doesn't have arithmetic so you can't subtract ten from it.

Why not use the ternary operator instead: if v.is_empty() { -9 } else { -10 }

Sure, although that's not a ternary operator, it's just an if expression.

In practice you tend to see such casts in code where we directly needed the integer. I think Clippy (Rust's linter) proposes them where you've written if some_bool { 1 } else { 0 } because well, that's what the as cast does anyway.

I find the if expression easier to read to be honest. With the integer cast example you need to mentally translate the booleans and do a subtraction in your head to figure out what integer values are involved, with the if expression it's directly visible.

Or you compile with -Wall -Werror, as you should do for serious projects.

I'm not saying C++ is a great language, it's not, but if you remove the secu then shoot you in the foot, it's your fault.

> Or you compile with -Wall -Werror, as you should do for serious projects.

-Wall -Werror does not catch the error described in the blog.

It does get caught by a static analyzer though [0].

[0]: https://gcc.godbolt.org/z/nK7MdEcMj

clang-tidy is still probably the best static analyzer for c++, g++ needs to catch up as so far its -fanalyzer is really just for c, not c++

I would add that adding lots of checks can cause clang-tidy to have serious performance problems.

The software I'm working on is safety-critical. I enabled every relevant checker relevant for certificates and tons of additional checkers related to usability and readability. clang-tidy now takes 3x the time it takes to compile :S I ended up splitting off a few checkers in particular into their own separate job.

Some checkers are also quite broken. bugprone-unchecked-optional-access is very broken. It caused hangs, it caused crashes, it caused out-of-memory issues. Fortunately, our use of std::optional can mostly be refactored away but doing so is a significant change in the codebase.

You may also need -Wconversion

Linters like cland-tidy help a lot with finding implicit bool conversions, among other common mistakes. Could even hook to patchset-received to ensure linters are passing ;)

Is it possible to enable a warning in C++ for these cases?

Given that there is a multitude of C++ compilers: Depends on your compiler. But in general, most compilers have a warning for this sort of thing.

1. Not in GCC.

2. This would be caught by static analyzer

See Compiler Explorer example here [0]

[0]: https://gcc.godbolt.org/z/nK7MdEcMj

If you want, you can disallow implicit conversion between boolean and number using a linter.

I imagine that you'd get a load of false positives due to textual inclusion of header files.

Compilers have no trouble ignoring header files outside of your project for the sake of warnings with -isystem instead of -I, it's not different for linters

> Compilers have no trouble ignoring header files outside of your project for the sake of warnings with -isystem instead of -I, it's not different for linters

In this [0] example I shared in other related comments, the static analyzer suppressed warnings in such headers.

> 126 warnings generated.

> Suppressed 124 warnings (124 in non-user code).

[0]: https://gcc.godbolt.org/z/nK7MdEcMj

Any linter that gets confused by includes would be rather useless, it couldn't even print the correct line numbers for any errors it finds.

Or with -Wimplicit.

that's only for c, not for c++

real-world cpp lives together with static analysis, which obviously catches this

Reply on news.ycombinator.com