Hacker News Reader

Comment by bragr | Hacker News Reader

I don't know anything about the code quality of the author, but in my experience, the ones that complain about static checking and other guard rails the most, are the ones that need it the most, because in their hubris of thinking they won't make those kinds of errors, they make the most egregious forms of those errors. e.g. smart enough take pot shots about the possible downsides of the process, but not smart enough (or self aware enough) to see their own fallibility.

View on HN

I remember an argument with an engineer about static code analysis years ago. Without a trace of irony, he honestly said "I know it's high quality code. I wrote it. What could a program tell me about it that I don't already know?" You can guess how much of a nightmare his code was to work with.

This isn't a comment on the original article or the author at all. Just my experience with a certain type of engineer.

There must be a corollary for programmers who have their preferred set of static analysis tools, but bristle in the same manner at new static analysis tools, or other peoples' static analysis tools.

"Have you ever noticed that anybody driving slower than you is an idiot, and anyone going faster than you is a maniac?" ~ George Carlin

I'll give a legitimate example I've had with Python and Mypy. The codebase I work on uses static typing extensively, and largely it's great. However Mypy is... well it fails open badly in obviously incorrect cases. The problem is the teams are by now socialized to trust Mypy and get surprised when very obvious things fail in production because they thought the tool would catch it.

This is the real risk of static analysis IMO, putting undue trust in the tool(s).

I don't know if this is the same phenomenon, but it's a good lesson on its own. No testing or analysis tool is perfect all the time.

That said, I'm curious about the "fails open" cases. Are you using `--strict`?

We can't use strict because typing was introduced post-hoc and while most of the codebase is covered, not 100% is.

That’s more a problem of moot and python than of static analysis itself.

Python is a dynamic language and mypy is as an after thought not a sound static type checker.

Better than nothing…would be way more risky not having it. However engineer must understand the shortcomings.

I btw gladly switched from python to Kotlin. Makes it so much easier to trust the compiler. Less work for me

This guy makes up 90% of the HN comments about how safe C is possible, you just have to pay attention.

hahaha, my inclination is almost always the exact opposite. Half the things I wrote a year ago are shit and need to be rewritten and I know it and so do all of my users

In my experience, the people who develop static analysis tools and add warnings to compilers are aware of the problems. They know that turning on all warnings is the path of insanity.

It’s a knob that you turn. You get useful information as you start to raise the warning level, but as you increase it higher and higher, you start to spend more and more time managing and silencing warnings at specific locations in your code, or dithering about how to handle some particular warning that does not (or even can not) manifest as actual incorrect program behavior.

I’m the first person to enable Wall Wextra and a few others, and I even turn on Werror religiously during development. But there are some super noisy warnings around. Static analysis tools are wonderful but they usually contain rules that are unreasonably picky, for most people most of the time. Usually these rules are off by default.

I read this article not as complaining about static checking but as warning that a rule that compilation should not produce any warnings can be counter-productive if many potential problems that get reported are red herrings.

As an extreme example for C, if you had a compiler that issued a warning for every potential int overflow, I think every programmer would ignore those warnings because it would report a warning on such statements as

  int foo = bar + 1;

and preventing that requires one to write

  int foo;
  if(bar != INT_MAX) {
    foo = bar + 1;
  } else {
    ???
  }

Even ignoring that figuring out what to write for ??? can take a lot of time, that blows up the code. Macros can make the source smaller, but your binary will still be larger and slower.

This is actually my annoyance with monadic error handling. When I'm writing a little command line utility, I don't really want to explicitly handle every edge case. Just let the damn program crash and shame on me.

At least with Rust's take on monadic errors, you can sprinkle some `unwrap`s around and get what you want.

In better languages, the compiler will write that code for you, and eliminate it whenever it can prove that the condition will never be hit.

This is one of the philosophical differences between C and other languages that makes everyone consider C unsafe. A legacy language that should no longer be used, because all of its users are perpetually incentivised to take shortcuts precisely like the one you've just described.

Most programming languages are not expressive enough to reliably eliminate redundant overflow checks (or overflows); afaik doing so in general way requires something in the ballpark of refinement/dependent types

In which case the compiler will emit the overflow or bounds checks, which is fine.

If the developer is certain that a specific add operation should use unchecked add for performance, then they can override the behaviour as a spot micro-optimisation. That then becomes a "self documenting" thing, easily found by a simple keyword search.

To give you an idea of just how rarely this is actually needed or done, there are less than a thousand uses of "unchecked_add()" in all of the Rust code on GitHub!

https://github.com/search?q=.unchecked_add%28+language%3ARus...

If you don't know what ??? should be, your code's going to misbehave in production - at best crashing, at worst causing data corruption or an exploitable vulnerability.

If "???" is just "foo = INT_MIN;" clang will optimise it all down to a single addition (see https://godbolt.org/z/K1ee8h3TM). If it's always "foo = INT_MIN;" across all of your code you can just use "-fwrapv" to override the C standard and define signed addition as two's complement wrapping.

GCC will ignore the possibility of overflow in signed addition if you don't do anything special (e.g. https://godbolt.org/z/Pc76c5E9z).

You can use "-ftrapv" (or "-fsanitize-undefined-trap-on-error -fsanitize=signed-integer-overflow") to make bigger, slower, safer code that fails fast on unexpected overflows across the board.

You can do the rather unergonomic "__builtin_add_overflow(bar, 1, &foo);" to tell the (GCC or clang, at least) compiler "???" is two's complement wrapping. You can do the cheeky, non-portable, but predictable on the same compiler and platform "int foo = bar + 1L;" to do the same thing. Combine these with "-ftrapv" and you'll get code that short-circuit blows up for unchecked unsigned overflow with the opportunity to produce faster code where you either are damn sure it can't overflow or where you want two's complement wrapping on overflow.

If it's not in a tight inner loop avoiding a check is premature optimisation - if you're consuming I/O and doing lightweight processing on it, an overflow flag test on signed additions will not make a measurable performance difference, nor will it blow out your binary size unreasonably.

IMO it's not a red herring to have signed integer overflow flagged as a potential error. The error is the failure to have an answer for "???".

> If you don't know what ??? should be, your code's going to misbehave in production

Not necessarily. The programmer may know the ??? code will be dead, but it’s not realistic to expect the compiler to deduce that.

For example, the programmer may know that, in this program, baz has been checked to be in the range [0,999] ten layers up the call stack, and bar was computed from it five layers up by calling quux, and thus will be in the range [31,98355], which is fine, given that this program tests that int can hold that range at startup.

Not all functions in a program have to be free of undefined behavior in isolation to make a program free of it.

That's a great example of knowing exactly what ??? means: the program is misbehaving and should terminate.

It's dead in the current call stack, but over time programs change and that is how errors creep in. If it's a precondition that _baz_ is in a particular range for the function to behave, why not check that in an assert and save yourself or a future programmer a ton of stress trying to work out why some unrelated part of the code blew up when they made what seemed like a simple change nine layers up the call stack?

There's no runtime cost to an assert in production code, but it makes your intent clear to both the compiler and other humans.

What level of static checks are you comfortable with? Because I don’t exactly think you’re wrong, but writing deviation reports for nearly every cast is a bummer, and I think many who have worked with high levels of static checks are familiar with the situation where there is no possible way to write a line of code that does not cause at least one message. I’ve spent days writing justifications for disabling particular messages (justifications such as “analysis tool authors misunderstood misra-c, here is evidence” or “reason for message is based on single paper from 1978, does not align with programming practice of the past 30 years”).

I’m net positive on static analysis, but I will forgive a lot of grumbling. It’s a lot of effort and does not catch many issues.

Furthermore, point 4 in the article (fixing unimportant warnings can cause bugs elsewhere) is real. I mean, it indicates issues with test coverage, but who doesn’t have issues with test coverage? Oh, 100% unit test coverage? Is that branch coverage or MC/DC coverage?

But anyway, on one team, when making warning fixes we would compare the final binary to show that with our compiler (the one that matters) our change produced identical output to the previous commit. If there actually was a bug that the warning found, well that was a whole different procedure.

OTH: If the warnings can't have false positives, they would be errors.

There's a lot to talk about 'warning hygiene', it's part of the daily life of C/C++ coders since it's very common that existing 'warning clean' code bases suddenly spam new warnings after upgrading compilers to a new version (that's also why it's good advice to not lambast an unfamiliar code base for having warnings before investigating why that is, it may simply have been developed with a slightly older compiler version than yours).

I don't agree with the author though: warnings should be turned to 11, then investigated, and locally suppressed if they turn out to be false positives (which is very frequent if you turn the warning level to max - also because some of the 'fringe warnings' are merely coding style suggestions).

Also: static analyser tools almost always depend on additional hints outside the language spec (usually provided through asserts), without those additional hints all static analysers produce a ton of false positives because C and C++ alone don't provide the builtin language features to express enough 'intent' to the analyser.

Don’t 100% agree with this, but it reminds me of a former coworker who got annoyed by all the “noise” of warnings (in a C++ codebase) so he set the flag warn=off. When I turned them back on, there were over 3000 warnings. Including one about a format string error that literally said “format string does not match arguments, this code will crash if executed.”

Warnings should be checked by CI. Adding a new warning should fail the build requiring you to either deal with it, or add a disable comment to the section.

Otherwise warnings are useless.

Wow! I was about to comment: when there are so many unheeded “noise” warnings, it can be easy to miss rare ones and new ones.

I remember a codebase with thousands of warnings, and hiding among them were a few with the word “intend”, like:

“if (condition); statement;”

https://learn.microsoft.com/en-us/cpp/error-messages/compile...

“variable == value;”

https://learn.microsoft.com/en-us/cpp/error-messages/compile...

There's a link to Professor Lemire's bio on the linked article: https://lemire.me/en/

He's good to very good when talking about performance and simd tricks but I don't know if he's particularly sage when it comes to general programming practice.

In this case I think he's probably wrong about compiler warnings but basically right about a lot of static analysers.

I'm not sure of your familiarity, but I've worked with Daniel on several projects, and think you might be guessing wrong about his attitude toward warnings. He's not the sort who ignores warnings. His tendency is to change his code so they don't happen, even if he doesn't think a particular warning is helpful or necessary.

I'm pretty sure his point isn't that the end user should get in the habit of ignoring warnings, but that those who decide which warnings are produced at which level need to be conscious that adding too many false positives can be counterproductive. His target audience here isn't application programmers, but those who decide which warnings are made default.

Hmm...when the memory checker came out for Xcode, I ran it on my code-base, parts of it decades old.

I still haven't adopted ARC.

I expected mayhem.

Turns out it "found" two false positives, i.e. leaks that are mandated by the low-level Objective-C APIs. I put the required incantations around them to shut up the checker and then I was good to go.

Keeping it simple, I guess...

¯\_(ツ)_/¯

Oh, and back to my thesis about "safyeness"[1], I certainly felt much better about my code once those checks had passed, even though it hadn't changed.

[1] https://blog.metaobject.com/2014/06/the-safyness-of-static-t...

There's an overwhelming bias in the profession towards writing code that creates some kind of result first, and then figuring out what it does later.

I don't think it's new or specific to tooling practices, it just reflects seasonal fashions based on whatever the last problem someone had was: "the old stuff was too restrictive" vs "the old stuff blew up too often". And there is always difficulty at the beginning of the specification in that feature design tends to change partway through, so guardrails in implementation can hinder the path towards seeing the result.

However, I do think it's countered in some sense by designing towards intentional wrongness: In my code it's always been habitual to first write skeleton code that is a stub that always "returns true", prints out string literals that are what I want, or other such things. Where something a little more complex like managing the state of a video game scene is called for, I've learned to turn towards first enumerating a large number of states, and then gradually parcelling it out into more specific data and algorithms. The reason why that works is because it means I am starting from a fundamentally static position: I've exactly specified what states the system can be in, and haven't taken on unexpected surprises. So I know that I don't cover everything, but I do understand what I covered.

In contrast I think there's also a mindset of "open-ended permissive, fix as you go" where the system is allowed to do anything at anytime, and features are shipped broken and maybe discovered in test. I believe this is what results in ball of mud designs because, with no central point of enumeration, it allows the specification to creep up from any direction. But - it gets a result. And as the system's ambitions grow, the temptation to throw open the doors of permissiveness also grow. And that leads to the "alternate hard and soft layers" pattern[0]. Where we fail isn't in not using that pattern, it's in not recognizing when it's forming and heaping too many concerns on a specific layer.

[0] https://kidneybone.com/c2/wiki/AlternateHardAndSoftLayers

Pretty sure his code quality is good. See https://lemire.me/blog/about-me/

Anyway, this is mostly a warning to more junior programmers.

Reply on news.ycombinator.com