5 ms·
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 i
by cesarb 4y ago
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.
- LudwigNagasena 4y agoIf you want, you can disallow implicit conversion between boolean and number using a linter.
- quietbritishjim 4y agoI imagine that you'd get a load of false positives due to textual inclusion of header files.
- jcelerier 4y agoCompilers 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
- inetknght 4y ago> 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 https://gcc.godbolt.org/z/nK7MdEcMj
- josefx 4y agoAny linter that gets confused by includes would be rather useless, it couldn't even print the correct line numbers for any errors it finds.
- orwin 4y agoOr with -Wimplicit.
- synergy20 4y agothat's only for c, not for c++
- tester756 4y agoThat's the way to go. But it is sad that in $current_year, we're still fighting boring, solved problems.
- toxik 4y agoMeanwhile in Python land, this is not a common class of bugs even though this implicit boolification happens.
- lostmsu 4y agoMy only explanation for that may be because Python has more severe and commonly occuring issues?
- cma 4y agoI'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.
- cjbprime 4y agoI wrote a (len(foo == 0)) instead of (len(foo) == 0) in Python yesterday. I don't understand why you would say this.
- toxik 4y ago… which is exactly what you don’t need to do in Python, because that is just `if foo`, thus sidestepping the issue
- mr_00ff00 4y agoLol 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.
- diarrhea 4y ago
- mentos 4y agoIs it possible to enable a warning in C++ for these cases?
- TonyTrapp 4y agoGiven 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.
- inetknght 4y ago1. Not in GCC. 2. This would be caught by static analyzer See Compiler Explorer example here [0] [0]: https://gcc.godbolt.org/z/nK7MdEcMj https://gcc.godbolt.org/z/nK7MdEcMj
- s9w 4y agoreal-world cpp lives together with static analysis, which obviously catches this
- orwin 4y agoOr 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.
- inetknght 4y ago> 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 https://gcc.godbolt.org/z/nK7MdEcMj
- synergy20 4y agoclang-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++
- inetknght 4y agoI 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.
- knorker 4y agoYou may also need -Wconversion
- NoOneSpecialII 4y agoIf only there was a language that defines explicit conversions between booleans and numbers.
- remram 4y agoDon't they all have == and != though?
- tialaramex 4y agoSure, 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.
- remram 4y agoWhy not use the ternary operator instead: if v.is_empty() { -9 } else { -10 }
- tialaramex 4y agoSure, 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.
- alpaca128 4y agoI 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.
- deleted 4y ago[deleted]
- Smar 4y agoLinters 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 ;)